Sanctuary: Reasonable defaults for env

Created on 13 Mar 2018  路  10Comments  路  Source: sanctuary-js/sanctuary

Correct me if this is the case already, but env configuration should be something only needed if we're indeed customizing. As it stands, disabling type checking in production or any other common uses along those lines should be default.
Libraries like React do this automatically and I believe Sanctuary should too.
Again, apologies if it does this already, in which case the documentation is unclear.

Most helpful comment

Well argued, folks. You have changed my mind. Have a look at #512 when you get a chance. :)

All 10 comments

Are you suggesting that require ('sanctuary') should produce a Sanctuary module which does not perform type checking?

It should figure out whether is running in production and disable it, unless explicitly opted-in by the user creating its own env. React, Webpack and others has such sane defaults.

I'd rather have type checking on by default and have a production build of sanctuary-def with the type checking code removed (to save file size).

It should figure out whether is running in production and disable it, unless explicitly opted-in by the user creating its own env. React, Webpack and others has such sane defaults.

I don't consider that to be a default, since the value is context-dependent. I consider that to be magic. ;)

I remain happy with our response to sanctuary-js/sanctuary-def#6. I'm open to changing the default value of the checkTypes option from true to false, if there are compelling reasons to do so.

You might not realize these as authors, contributors or people who had been using this for a while, but I ought to say the barrier of entry to this library is really high, and not because of FP but because these kind of decisions and obvious omissions.

In the JS ecosystem (and others, but can't speak for all let's keep it simple) I would expect a library, any library, to give me extra warnings on development, and remove those if I'm running on production by default. NODE_ENV exists for a reason, what's really the argument against using that? A shared var? It's meant to be, babel uses it, your app uses it, webpack uses it.

Does it couple it to the browser, no it doesn't, most people are bundling and you can safe-check on process. I'm not really getting it.

If I want type checking in production then I would go and create my own Sanctuary env.
It's how Webpack works, React, flow-runtime even, the latter is essentially type checking BTW.

Am I really tripping here? I don't see the magic. Type check on on development by .default, off on production by default. Or provide an entry with it disabled I can at least alias.

I've enough stuff to configure and I'm sure you do as well, reason (not ml) please. 馃槀

the barrier of entry to this library is really high

Once one reaches the point of needing S.create, one may be required to learn about sanctuary-def Type values. This could feel overwhelming. @Avaq has given considerable thought to how to make the set-up smoother. We would love to reduce friction in ways which do not compromise our values.

People trying Sanctuary for the first time should not hit the S.create barrier. I consider this a pleasant introduction to a library:

$ npm install sanctuary
$ node
> const S = require ('sanctuary')
undefined
> S.toUpper
toUpper :: String -> String
> S.toUpper ('foo')
'FOO'

NODE_ENV exists for a reason, what's really the argument against using that? A shared var?

Tying the behaviour of unrelated functionality to a shared environment variable is problematic, in my experience. Perhaps I want to test against the staging database so I update NODE_ENV, only to realize that log messages are now being sent to Papertrail rather than to my terminal. Flexibility is valuable.

If you would like Sanctuary's behaviour to depend on NODE_ENV, you could write a tiny wrapper to achieve this. When I worked at Plaid, we had a private plaid-sanctuary module similar to this:

var S = require('sanctuary');

module.exports = S.create({checkTypes: process.env.NODE_ENV !== 'production', env: S.env});

People trying Sanctuary for the first time should not hit the S.create barrier.

No, instead they'll hit the "my performance has been degraded by a factor of at least 2x" barrier.
Which, you'll have to agree is more intrusive to my app than the library assuming I use NODE_ENV as the rest of the JavaScript ecosystem does.

If you would like Sanctuary's behaviour to depend on NODE_ENV, you could write a tiny wrapper to achieve this.

The fact that people have to do this is what I've been trying to say it's plainly wrong.

@Avaq has given considerable thought to how to make the set-up smoother

That has always been in the face of your opinion about NODE_ENV, @davidchambers ;) I have to side with @blackxored on this one. I think using NODE_ENV in the definition of one of your default configuration vars is reasonable.

People trying Sanctuary for the first time should not hit the S.create barrier.

They will not, if we default the type checking to process.env.NODE_ENV !== 'production'. Assuming of course they are not playing with sanctuary in an environment they have explicitly set to "production".

Perhaps I want to test against the staging database so I update NODE_ENV, only to realize that log messages are now being sent to Papertrail rather than to my terminal.

There are two observations which invalidate part of this argument.

  1. Whether type checking is enabled or disabled should not alter the behaviour of our code (unlike Papertrail usage).
  2. I think it's the minority of users who would want to separate type checking behaviour from the NODE_ENV value. Those users are free to override the default NODE_ENV !== 'production'. I imagine you are already doing so in your own code anyway, because the current default value (true) is just as unwieldy to those users.

we had a private plaid-sanctuary module similar to this

Don't we all?

Well argued, folks. You have changed my mind. Have a look at #512 when you get a chance. :)

The default Sanctuary module now performs no type checking if process.env.NODE_ENV === 'production'.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

toolsh picture toolsh  路  6Comments

t1ger-0527 picture t1ger-0527  路  3Comments

arsaniwilliam picture arsaniwilliam  路  9Comments

blackxored picture blackxored  路  7Comments

yotamDvir picture yotamDvir  路  3Comments