Sanctuary: support esnext module format for tree-shaking purposes

[mrtnbroder]

It would be great if sanctuary could introduce a esnext version to support tree-shaking and a more modular approach.

[davidchambers]

I'm aware that this is possible, but I don't know any of the details. Can you recommend a resource for learning about this, @mrtnbroder?

[mrtnbroder]

If you want, I could give this a shot myself. It's actually not that 'hard' to do, just tedious. But the effort is worth it.

Actually, it's just looking at the modularity of your project and splitting parts of them into their own file/section, so one could import maybe from 'sanctuary/lib/maybe directly as an example.

It's a bit cumbersome to do this in es5 syntax, so I'd probably go the way of adding babel and a little prepublish script that produces an esnext and an es5 version of this lib.

if you want to learn about the module format in general I can recommend this section of "Understanding ES6" https://leanpub.com/understandinges6/read#leanpub-auto-encapsulating-code-with-modules

[mrtnbroder]

Take a look at https://github.com/reactjs/react-router for a pretty good example of how this can be done, @davidchambers !

[rjmk]

@mrtnbroder I may well be wrong on this, but is it true that

a) It's as easy to split the file up and do require('sanctuary/maybe') in commonJS
and b) It's not necessary to split the file up to get the benefits of tree-shaking, but rather the move to the static exports of ES6 is what does the work?

My understanding is that splitting into files and doing the require('sanctuary/maybe') allows people to pick 'by hand' any code they wish to bundle and the exports allow automated tools to figure out what code you are using

[davidchambers]

I do :heart: the lack of any build step in this project, but I do appreciate the advantage this change would provide.

If you want, I could give this a shot myself.

I'd love to see a proof of concept. Perhaps you could put together a patch to show this working for a handful of functions.

This is all new to me, @rjmk, but I'm very interested to know whether your understanding is correct.

[rjmk]

@davidchambers I'd like to echo your sentiments on the lack of build step

[mrtnbroder]

It's not necessary to split the file up to get the benefits of tree-shaking

Correct. It just requires the export syntax.

@davidchambers whats wrong with a build step? It doesn't necessarily require a build tool like gulp or something. Babel would be enough to compile to ES5. It's a one-liner in the package.json file.

Alright then, let me see if I can provide a simple patch. Will take a bit though, I am a bit busy atm ;)

[davidchambers]

whats wrong with a build step?

One can make things reasonably pleasant. Let's say one's using Make. One can have make test run make to ensure that one's tests always run against the latest build. But perhaps the tests are slow to run (as they are in Sanctuary's case). One may then wish to run

$ node_modules/.bin/mocha -- test/someFunction.js

to test a specific change. One may be surprised that the tests are still failing. Oh, we forgot to include make && at the start of the command!

Another issue is that Make relies on file modification dates, which is sometimes problematic when changing branches. So sometimes one finds oneself needing to run

$ rm -r dist && make && node_modules/.bin/mocha -- test/someFunction.js

to ensure that the JavaScript file one is testing is up to date.

These problems are minor, but losing five or ten minutes figuring out why changes are not taking effect does negatively affect my attitude towards a project.

Another source of complexity is how to handle the generated files. One problem I encountered with CoffeeScript projects was that contributors would sometimes include changes to the generated file in their patches, even if the generated file was intended to be updated only at release time. This can be avoided via careful use of git update-index, but this adds another possible source of confusion: changes to the generated file are not visible when one runs git status.

In Ramda's case, there's another source of complexity caused by the use of a custom build script. We must test both require('ramda').someFunction and require('ramda/src/someFunction'). On at least one occasion an incorrect require path meant requiring a certain function did not work, though the function behaved correctly when imported as part of the 'ramda' module.

None of these problems is insurmountable, but having spent many hours working with JavaScript projects with build steps over the past several years, working on Sanctuary is a breath of fresh air.

[mrtnbroder]

usually you add any build output to the .gitignore so people change the source and not the output code. You only get the output code when you npm install the package.

[mrtnbroder]

Also, you can just export { all, these, tasty, functions } at the index.js so you can import everything from the index.js file. One should actually never explicitly import a function that was intended to be private from the source directory, only from the public api.

So no import someFunc from 'sanctuary/lib/someFunc'
but rather import { someFunc } from 'sanctuary'

[rickmed]

:+1: this!

[mrtnbroder]

I'm on it!

[mrtnbroder]

Almost done 😍

[mrtnbroder]

I have one major problem with the create function though @davidchambers I'm not sure how to support it now where everything could be imported directly via import { T } from './sanctuary', where T and everything else is already defined by a default def function

One idea that comes into mind is that the checkTypes prop will depend on the NODE_ENV being either production or not. So type checking is there when you are in dev mode, and not when you make a production build.

Another idae would be to add a flag like --check-types so it does check types on runtime, and not if you do not provide this flag.

I think the best idea would be though to completely get rid of the S.create function and let people define their newtypes within a separate file, and do not pollute sanctuary with their stuff. It doesn't make a lot of sense to me either way. One should not extend jQuery for example with their own functions 'just like that', but rather write a plugin for it.

I hope it makes sense to you what I mean, I'll push a WIP version in a moment to my fork: https://github.com/mrtnbroder/sanctuary

[davidchambers]

I think the best idea would be though to completely get rid of the S.create function and let people define their newtypes within a separate file, and do not pollute sanctuary with their stuff.

The require('sanctuary') module is never modified: S.create returns a _new_ Sanctuary module which has access to an environment via its closure. There's no pollution.

Ideally Sanctuary would not require knowledge of one's types, but #188 demonstrates that it does.

Many Sanctuary functions are polymorphic. Take concat :: Semigroup a => a -> a -> a, for example. How is it that we're able to provide the following behaviour?

S.concat('abc', [1, 2, 3]);
// ! TypeError: Type-variable constraint violation
//
//   concat :: Semigroup a => a -> a -> a
//                            ^    ^
//                            1    2
//
//   1)  "abc" :: String
//
//   2)  [1, 2, 3] :: Array Number, Array FiniteNumber, Array NonZeroFiniteNumber, Array Integer, Array ValidNumber
//
//   Since there is no type of which all the above values are members, the type-variable constraint has been violated.

The answer is that each time we encounter a value of type a we refine the possible values (types) of a. In order to do so we need an initial set of types to filter. Were we to restrict ourselves to built-in types, functions such as concat would not work on values of user-defined types. We could include Either and Maybe in the set of types, but concat would still not work on values of types defined in other libraries.

S.create seems necessary if we're to continue to type check applications of polymorphic functions.

[rjmk]

If we want tree-shaking and to play nicely with type checking, I think we would have to have an API change.

One could imagine that either (a) all the functions need to have an env passed into them or (b) create also took the set of functions it was to produce (this would be compatible with (a), it would just apply them to the environment)).

Then one would _need_ an internal Sanctuary file, where one would basically

import { _T, _either, ..., create, env } from 'sanctuary'

var myEnv = env.concat(...)

export var { T, either, ... } = create({ env: myEnv, checkTypes: true, functions: { _T, _either, ... } })

One could also imagine other, slightly different APIs. I think a better one would be

import { T, either, ..., create, env } from 'sanctuary'

var myEnv = env.concat(...)

export default create({ env: myEnv, checkTypes: true, functions: { T, either, ... } })

But I don't know how that plays with destructuring import

[mrtnbroder]

Why does sanctuary expose sanctuary-def's create anyway? I'm strictly about separating these from sanctuary. You should import { create } from 'sanctuary-def' and not from sanctuary imho.

[rjmk]

One could imagine Sanctuary taking in a def defined by the user with sanctuary-def. It could then use that throughout, but it still wouldn't support treeshaking (I think -- it would only be exporting the one monolithic function).

It could export all the functions with them waiting for a def. Then one would presumably do something like

import myDef from 'my-def'
import { map } from 'ramda'
import { T, either } from 'sanctuary'

export default (R.map(f => f(def), { T, either })

This might be a bit annoying when you're using most of Sanctuary, so it could also have a way of importing everything with the default environment. That might be the "Sanctuary" module and this stuff might be sanctuary-functions.

An entirely different approach might be to change sanctuary-def so that the wrapped functions had a method on them envConcat that can expand the environment for a function on a per-function basis

[davidchambers]

sanctuary-js/sanctuary-def#74 is highly relevant to this discussion.

You should import { create } from 'sanctuary-def' and not from sanctuary imho.

I don't think this would work. Here are two relevant snippets from Sanctuary's source:

var def = $.create(opts);

var I = S.I =
def('I',
    {},
    [a, a],
    function(x) { return x; });

S.I is defined via def, which is turn depends on opts, which specifies an environment. After defining S.I there's currently no way to retroactively "inject" types into its environment, so S.I is only able to operate on values of types in the environment provided to S.create.

I have a feeling there's a good solution out there. @Avaq and @rjmk have put forward several interesting ideas. Perhaps having each function take an environment as its first argument is the answer. One could then do something like this:

const S = R.map(f => f(env), require('sanctuary'));

It would be less pleasant, though, if requiring functions individually:

const head = require('sanctuary/head')(env);
const init = require('sanctuary/init')(env);
const last = require('sanctuary/last')(env);
const tail = require('sanctuary/tail')(env);

I'm not sure whether the above is something we want to support, though. I'm still getting my head around "tree shaking".

[svozza]

That seems very invasive; I'm not against modularisation but this treeshaking stuff seems to spiralling out of control, I'd rather leave things as they are than go with any of the solutions proposed so far.

[rjmk]

@svozza I definitely agree treeshaking is not a good enough reason to do this. It seems to me something like this is definitely worth exploring though for composable environments and good dependency management. Creating environments with which one can extend the default environment is definitely possible, but it's hard to create functions which depend on a given environment while keeping as easy an install story.

Perhaps having each function take an environment as its first argument is the answer.

I think I prefer taking def to taking the env. That then would stop as direct a dependence on sanctuary-def.

It would be less pleasant, though, if requiring functions individually

The API wouldn't have to be like that for 2 reasons.

1. I think there's still place for the functions applied to the default environment. If anything, I would keep sanctuary as the coordinating repo/module that comes 'batteries included'
2. You could have a central index.js file that exports all the functions. Either

// commonJS

module.exports =
  { I: require('./I.js')
  , S: require('./S.js')
  ...
  }

or

// ES6

export I from 'I.js'
export S from 'S.js'

Though in the ES6 case, I'm pretty sure breaking the files up isn't actually necessary.

This index file would be what one received from require('sanctuary')

[davidchambers]

@mrtnbroder, perhaps you could open a pull request for the changes you have made so far? Don't worry if there are failing tests and whatnot.

[mrtnbroder]

@davidchambers will do so asap. Need to refactor the commits a bit.

[mrtnbroder]

Sorry about the delay, much work, wow, not much time.

just wanted to inform you that I did not forget about it!

[davidchambers]

No problem, @mrtnbroder.

With at least two significant pull requests soon to be merged I just want to make sure you don't spend many hours on this only to find yourself with dozens of merge conflicts only a week later.

By the way, I'll be living in Berlin from tomorrow. If you'd like to get together one day to work on this (or anything else) in person, contact me via email or Gitter. :)

[davidchambers]

I'm still interested in this, but I'd like to keep the source code in a single file.

[davidchambers]

I haven't yet seen a clear plan presented. I'm open to making changes to reduce the size of production builds, but every approach proposed so far has at least one significant drawback.

[MikaAK]

I'm curious what the reasons for wanting to keep this in a single file are @davidchambers

[davidchambers]

@MikaAK, see https://github.com/sanctuary-js/sanctuary/issues/246#issuecomment-240986805 for my reasons for opposing a build step and https://github.com/sanctuary-js/sanctuary/issues/246#issuecomment-244220089 for the difficulties S.create would cause were we to accept the costs of a build step.