disable runtime checking without NODE_ENV=production

[gcanti]

From @marwanhilmi's comment here #165

Is there a quick method to disable runtime checking without needing to run in NODE_ENV=production? We use NODE_ENV to toggle our config, ie NODE_ENV=staging so it would be nice to have another option to disable runtime checking without specifically checking for production. Perhaps its own env flag?

[gcanti]

Perhaps its own env flag?

@marwanhilmi do you know any library doing that? I mean handling both NODE_ENV and a custom flag. And how? I'll do some research

EDIT: A strict requirement for any change on this matter is backward compatibility: that is if NODE_ENV=production then runtime checks will be disabled without additional burden for the users

[gcanti]

@marwanhilmi I have a doubt: when you say "disable" you mean to override the default bahaviour of throwing an error (which is already doable overriding the function t.fail) or to strip out entirely the runtime checks from the build (example for perf reasons)?

[marwanhilmi]

I didn't know you can override the t.fail method but yes I did mean both. I agree on the point that =production should still work without breaking.

Could we just check an additional variable in same place as NODE_ENV checks? I haven't dug into the code but more than happy to poke around.

[gcanti]

@marwanhilmi here my findings:

Quick fix (no code changes)

You can manually disable all checks overriding the t.fail function

// somewhere in your main...

import t from 'tcomb'

t.fail = () => {} // swallow all errors...

t.fail = (message) => console.error(message) // ... or just log to the console

ENV VARIABLES (code changes)

If the perf of tcomb is not acceptable in staging, we could add new env variables. Even so it's worth noting that other libraries, for example react, rely on NODE_ENV=production in order to boost their performance, so in staging you could end up with a suboptimal configuration despite the proposed additions.

In particular there are 2 possible reasons for perf degradation in tcomb:

1. runtime type checks (proposed variable name: TCOMB_DISABLE_CHECKS)
2. Object.freeze calls (proposed variable name: TCOMB_DISABLE_FREEZE)

Now, replacing all

if (process.env.NODE_ENV !== 'production') {
  ...runtime type check here...
}

...

if (process.env.NODE_ENV !== 'production') {
  Object.freeze(this);
}

with

if (process.env.NODE_ENV !== 'production' && !process.env.TCOMB_DISABLE_CHECKS) {
  ...runtime type check here...
}

...

if (process.env.NODE_ENV !== 'production' && !process.env.TCOMB_DISABLE_FREEZE) {
  Object.freeze(this);
}

should lead to the following outcomes:

# this ensure backward compatibility
NODE_ENV=production (regardless of TCOMB_DISABLE_CHECKS and TCOMB_DISABLE_FREEZE) :
checks: no
freeze: no

NODE_ENV=staging :
checks: yes
freeze: yes

NODE_ENV=staging && TCOMB_DISABLE_CHECKS=true :
checks: no
freeze: yes

NODE_ENV=staging && TCOMB_DISABLE_CHECKS=true && TCOMB_DISABLE_FREEZE=true :
checks: no
freeze: no

[rubengrill]

@marwanhilmi do you know any library doing that? I mean handling both NODE_ENV and a custom flag. And how? I'll do some research

@gcanti Babel has it's own ENV flag BABEL_ENV that is checked before NODE_ENV:
https://babeljs.io/docs/usage/babelrc/

[gcanti]

Hi @rubengrill, thanks for the link.

Checking an additional variable is easy, in the case of tcomb I must to ensure that the dead code elimination feature of uglifyjs will be triggered. After some tests this also seems to work fine:

if ((process.env.TCOMB_ENV || process.env.NODE_ENV) !== 'production') {
  ...checks...
}

Example

NODE_ENV=staging 
TCOMB_ENV=production

Result after the DefinePlugin (I'm using webpack for bundling)

if (false) {
  ...checks...
}

finally the UglifyJsPlugin will strip out the checks.

EDIT: I must check browserify/envify though

[gcanti]

Update: all the techniques seem to work with browserify/envify but only if the envify's option purge is used https://github.com/hughsk/envify#purging-processenv

[ivan-kleshnin]

I've heard performance issues with frozen values are no longer relevant in newer browsers.
So one option is to remove these checks alltogether.

P.S.
Sorry – can't find an exact link / thread with that information.

[nicolashery]

I saw something about checks using process.env being slow in Node, because it is not a regular object? facebook/react#812 Maybe something to keep in mind...

By the way, are you thinking of adding benchmarks to this repository? :)

[gcanti]

Hi @nicolashery,

are you thinking of adding benchmarks to this repository?

A first attempt (yesterday I was playing with the benchmark lib):

https://github.com/gcanti/tcomb/blob/benchmark/perf/perf.js

(for what concerns perfs I'm a noob so any help is appreciated)

[nicolashery]

Thanks @gcanti! I'm also completely new to benchmarking, so afraid I won't be much help :-/ Maybe looking at existing examples might be useful (for instance https://github.com/marko-js/templating-benchmarks, they use matcha which should be similar).

I see you're benchmarking the build tcomb.min.js. I guess that works for the browser, but what about a Node environment? One benchmark could be run using NODE_ENV=production on the source code. But also see the issue I linked to in my last comment, about caching process.env.NODE_ENV to improve performance (since it's not a regular object and accessing it can be slow for performance-sensitive code).

I would also suggest running a benchmark with a bigger data structure? (closer to a "real" HTTP API response, for example: https://github.com/marko-js/templating-benchmarks/blob/master/templates/friends/data.js).

[timoxley]

Have you considered memoization?

In any heavily tcomb'ed API, a single value may be checked many dozens of times by the exact same validation procedure. If nothing has changed, then this probably leads to a lot of wasted effort.

Perhaps verification results for Object types could be cached in a WeakMap? Seems like a no-brainer if the object being checked is also frozen.

[fatso83]

@timoxley you need to open a separate issue for that. This issue has nothing to do with your proposal, although a good one. It will go unnoticed in this one. It is closed.