Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ramda more modular #1505

Closed
luizbills opened this issue Nov 8, 2015 · 43 comments
Closed

Make ramda more modular #1505

luizbills opened this issue Nov 8, 2015 · 43 comments

Comments

@luizbills
Copy link

my proposal

var R = require('ramda'); // import ALL ramda functions
var compose = require('ramda/compose'); // import JUST the compose function
...
@davidchambers
Copy link
Member

require('ramda/src/compose') currently works, but it would be nice to use the cleaner path.

@luizbills
Copy link
Author

That's the point. 👍

@CrossEye
Copy link
Member

CrossEye commented Nov 9, 2015

👍

We've discussed this before, but no one's gotten around to doing it. It should just be a matter of how the NPM package is built.

@luizbills: Do you feel up to taking a swing at it?

@luizbills
Copy link
Author

This allows custom builds easy. Ramda is very big. Hardly ever someone will use the entire library.

@Hypercubed
Copy link

Any thoughts on converting to ES6 modules and using rollup? Tree-shaking Ramda with rollup would be amazing.

@CrossEye
Copy link
Member

CrossEye commented Dec 6, 2015

@Hypercubed: Could you describe the advantages? I haven't tried rollup yet.

Although it's not well-documented, it's fairly simple to build your own version of Ramda right now. This doesn't start with static analysis of a codebase, though, but with a list of public functions you want to include. The build does the recursive dependency management and builds as small a package as possible.

@Hypercubed
Copy link

Using ES6 modules would improve static analysis of the code base.... something that is difficult with CJS modules. On your end Rollup would do pretty much what you described... take an index file and build the smallest a package as possible (maybe replacing your build script).

I guess the advantage of using ES6 modules would be to allow users to do tree shaking on their end. They can import ramda methods individually, without the need to know the internal paths, then use rollup tree shaking to get a minimal build.

For example in my project I can do:

import {compose, isNil, equals} from 'ramda';

to get only the relevant portions of the ramda codebase.

Because Ramda is already pretty modular it's probably not to hard to convert to ES6 modules. The hard part would be converting the build process.

@CrossEye
Copy link
Member

CrossEye commented Dec 7, 2015

I don't think it would be hard to write a new build process to handle that. It would clearly work fine for browser usage. Even there, I have concerns about readability. I think of ramda.js as our flagship output. As much as possible, I would like that to remain as clean and readable, and as much like hand-crafted code as possible. But it's not at all clear that this would work for Node users. Right now Node users can use the same file that browser users do, and they can also import individual functions directly. (There is an unfortunate 'src' in the import path, but we can straighten that out..) If we switched to ES6 modules, would Node users would presumably have to run some build step before using the individual imports, and that seems to bad.

@Freyert
Copy link

Freyert commented Dec 17, 2015

Perhaps a feature branch could be started for the ES6 import. That way when it becomes the de facto it won't be but a hop skip and a jump for Ramda. Also, users who are just plain desperate for it right now would be able to clone from that branch.

I've proposed ES6 modules before, for the same reason @Hypercubed has. Perhaps we could tag team?

I think also with the rapidity that javascript is evolving it is probably going to be necessary to incorporate a transpilation step at some point. Hand crafted code is the best, but transpiled code would allow ramda to support a wider range.

Also, ES6 support could help Ramda dramatically improve other areas such as error handling. R.compose throws some fairly enigmatic errors if a function has issues, which makes it a hard sell to me coworkers. Function.name property would make it a more simple process by allowing us to throw the function name for any of the composed functions. (I could also just be doing this wrong.)

@scott-christopher
Copy link
Member

I was messing around with ES modules in Ramda the other day, so I figured I'd push a branch up that people can experiment with: https://github.com/scott-christopher/ramda/tree/es-modules

Note: this is just a experiment, with no guarantees it will eventuate into anything

The UMD module generated by rollup is surprisingly not far off what the current Ramda build script produces: https://github.com/scott-christopher/ramda/blob/es-modules/dist/ramda-rollup.js

@CrossEye's point about continuing support for importing individual modules in commonjs environments still stands, meaning if we were to go down this path we would need to look into deploying separate packages to NPM to support both module types. That could be a good opportunity to solve for being able to import individual modules in commonjs environments while we're poking at it.

@davidchambers
Copy link
Member

The UMD module generated by rollup is surprisingly not far off what the current Ramda build script produces

You're right! This is good to see. :)

@CrossEye
Copy link
Member

@scott-christopher: This is great! I'd love to be using external tools for this rather than maintaining our own. All kudos to @davidchambers, whose script was much better than the one I was building, but I'd still rather work with a tool that's well-maintained, if it does as good a job as this.

@Hypercubed
Copy link

@scott-christopher It looks great. What else needs to be done? Might also consider adding Babel to the mix to allow proper ES6 code.

@borisirota
Copy link

Hi guys, this is really great!

I just want to add that its possible to add to @scott-christopher es-modules branch the following steps:

  1. place index.js inside src dir.
  2. run babel src --out-dir lib and use the babel-plugin-transform-es2015-modules-commonjs plugin (maybe rename the src directory to es and then make babel es --out-dir src for backwards compatibility for guys importing specific functions like ramda/src/compose).
  3. change main field in package.json to "main": "src/index.js"
  4. add to package.json "jsnext:main": "es/index.js"

Check out Redux as example. They develop with es6 features so they use additional babel plugins for the transpilation.

The benefits are (you mentioned almost all of them):

  • Browser users - use rollup's output and rollup is the build tool.
  • Node users - same way like today but behind the scenes it will be separate modules.
  • Browserify\Webpack users - same way like today with the same ability to require separate functions.
  • Rollup\Webpack 2 users - can use tree shaking with jsnext:main (webpack 2 will support it before release)
  • @CrossEye - No build step from users and no need to deploy separate packages to NPM to support both module types.

I just started using ramda and its awesome !
Currently, the library size is something to think about before using it in web project. With this approach there is nothing to think about :)

BTW, babel-plugin-ramda (I made PR which adds babel 6 support) is a workaround for guys using babel in their build but it will be great to have it out of the box.

@CrossEye
Copy link
Member

CrossEye commented Feb 6, 2016

The holidays intervened and everyone got distracted by shiny new things, but we really should get back to this. This would be a fantastic improvement.

I suppose we need to keep backward compatibility with ramda/src/compose for a version or two, but I'm not worried about it for the long haul. The new approach is so much better, and the migration is very easy.

@scott-christopher: What do you think about trying to merge this for v0.20.0?

@scott-christopher
Copy link
Member

@borisirota suggestions sound like a good approach. I'll see if I can spend some time on it this afternoon.

@scott-christopher
Copy link
Member

I've updated my branch with the latest changes from master and made the changes suggested by @borisirota, though babel unfortunately adds a bit of unnecessary cruft to the generated commonjs modules (see example below), so I ended up just creating a simple shell script to do the transformation instead which can be run via npm run build:cjs

https://github.com/scott-christopher/ramda/blob/es-modules/scripts/build-commonjs

Below is an example of the babel-generated add.js commonjs module. Note that the plugin is declaring the export as exports.default rather than module.exports, which would break existing usage of require('ramda/src/add'):

'use strict';

Object.defineProperty(exports, "__esModule", {
  value: true
});

var _curry = require('./internal/_curry2.js');

var _curry3 = _interopRequireDefault(_curry);

function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }

/**
 * Adds two values.
 *
 * @func
 * @memberOf R
 * @since v0.1.0
 * @category Math
 * @sig Number -> Number -> Number
 * @param {Number} a
 * @param {Number} b
 * @return {Number}
 * @see R.subtract
 * @example
 *
 *      R.add(2, 3);       //=>  5
 *      R.add(7)(10);      //=> 17
 */
exports.default = (0, _curry3.default)(function add(a, b) {
  return Number(a) + Number(b);
});

Not really sure where we want to take it from here.

@borisirota
Copy link

Oh I totally forgot about the default thing. Well, its only needed for backwards compatibility so as long there is need for that you should use the babel-plugin-add-module-exports in addition to babel-plugin-transform-es2015-modules-commonjs to add back the module.exports (the order between the plugins important):

.babelrc

{
  "plugins": [
    "add-module-exports",
    "transform-es2015-modules-commonjs"
  ]
}

cli

babel es --out-dir src --plugins transform-es2015-modules-commonjs --plugins add-module-exports

Sorry for the extra work you had to do.

@borisirota
Copy link

The unnecessary cruft babel adds to the generated commonjs modules affects only browserify\webpack users -->

There are 2 use cases I'm familiar with:

  1. require("ramda") - to support this case we can add "browser": "./dist/ramda[.min].js" to package.json. the browser field tells these tools to take this file instead of the regular flow. Will take rollup's output without the extra bits.
  2. require("ramda/src/compose") - this is the only case which being affected by the extra bits. as @CrossEye said, it won't be available in the near future so it not seems like a big deal.

** EDIT **
I'm taking back what I wrote in 2. The source code will remain modular so it still will be possible to require specific modules this way (maybe src will be renamed as @CrossEye said if I understand it correctly).
So the extra bits will affect browserify\webpack users if using the babel plugins I proposed. In that case @scott-christopher shell script is a better solution.
I just want to mention that Rollup\Webpack 2 users using the babel-plugin-ramda can use the specific modules approach without the extra bits burden (the plugin can point to the version with es6 modules and the tools will do the rest without transpilation) in case you decide to choose the solution with the babel plugins and not the shell script.

@mjrussell
Copy link

Super excited about the progress being made on this issue. The bundle size reduction with webpack is something I would use immediately.

One thing to consider after releasing the support for individual function import might be a babel plugin for auto modularization. Lodash has a useful one:
https://github.com/lodash/babel-plugin-lodash

This would allows users to import ramda and use it normally, but transform their code into the 'modular' version.

@borisirota
Copy link

@mjrussell I mentioned before the babel-plugin-ramda. My PR with babel 6 support was merged few hours ago :)

@mjrussell
Copy link

@borisirota oops sorry totally missed it! 👏

@mAAdhaTTah
Copy link

Has there been any progress on this? Would love to get modular ES6 builds with webpack 2.0 getting tree-shaking. I'll definitely look into the babel plugin, but having that built-in would be even better. Any place I can help out?

@CrossEye
Copy link
Member

@scott-christopher: Any thoughts on where to go from here?

@scott-christopher
Copy link
Member

I have very little time available to look into this further at the moment, though I wonder whether we might be better off keeping all the existing source files as they are and instead generate the ES6 module files on the fly before deploying to NPM. Then we could just add "jsnext:main": "es/index.js" to package.json for things like Rollup to make use of.

The following bash script should get close to that (I'm sure that someone more knowledgeable in the magical world of shell scripting could improve upon this) if run before the deploy.

#!/usr/bin/env bash
set -e

REL_SCRIPT_DIR=$(dirname "$0")
ABS_SCRIPT_DIR=$(cd "$REL_SCRIPT_DIR"; pwd -P)
PROJECT_DIR=$(dirname "$ABS_SCRIPT_DIR")

mkdir -p "$PROJECT_DIR/es/internal"

for f in $PROJECT_DIR/src/*.js $PROJECT_DIR/src/internal/*.js; do
  sed -e 's/var \(.*\) = require(\(.*\));/import \1 from \2;/g' \
      -e 's/module.exports =/export default/' \
      "$f" > "$PROJECT_DIR/es/${f#${PROJECT_DIR}/src/}"
done

find "$PROJECT_DIR/src" -type f -depth 1 | sed -e "s|.*/\(.*\)\.js|export { default as \1 } from './\1';|g" > "$PROJECT_DIR/es/index.js"

If anyone wants to explore this further, I'll gladly help where I can.

@scott-christopher
Copy link
Member

I think if we want to embrace more of ES6 with the current codebase, we should perhaps seriously consider tagging 1.0 soon to start developing a 2.0 branch with ES6 features as first class citizens, deprecating the require('ramda/src/*') approach in the process ... though that's worthy of a whole new discussion.

@mAAdhaTTah
Copy link

The jsnext:main would be a a reasonable stopgap, but first-class ES6 features would be ideal.

However, this currently is no index.js file in the src directory, so you could make the index.js file an ES6 module that just exported ramdas functions. From there, you would just point jsnext:main at it, and (ostensibly), a tree-shaking bundler would only pull in the functions you need, as the rest of the require functions get parsed like normal. Does anyone know if you'd get the tree-shaking benefits through an entry file like this? And would there be any potential conflicts with userland tools?

@scott-christopher
Copy link
Member

scott-christopher commented May 12, 2016

However, this currently is no index.js file in the src directory, so you could make the index.js file an ES6 module that just exported ramdas functions.

There is a line in the script above that takes care of that:

find "$PROJECT_DIR/src" -type f -depth 1 | sed -e "s|.*/\(.*\)\.js|export { default as \1 } from './\1';|g" > "$PROJECT_DIR/es/index.js"

I had a quick attempt at getting tree shaking to occur with a simple example, though couldn't get it to work using Rollup while importing the index file.

To test locally, I generated the ES modules in a local copy of Ramda using the script above within the scripts folder, added "jsnext:main": "es/index.js" to the package.json and then ran npm link $path_to_local_ramda_install in a separate directory.

The following were some various results of running rollup -i add10.js -f cjs.

Importing from ramda:

import { add } from 'ramda';
export default add(10);

Converted to the following without inlining the dependencies:

'use strict';
var ramda = require('ramda');
var add10 = ramda.add(10);
module.exports = add10;

Importing the index file via a relative path:

import { add } from './node_modules/ramda/es/index';
export default add(10);

Converted to ... the entire ramda source with something similar to module.exports = add(10) at the end.

I also tried different formats of the index file to see whether that would help like having separate import and export statements rather than the export { default as add } from './add' } format, though no luck.

So in summary, I'm happy to help this along, though I don't have much experience with tree shaking using either Rollup or Webpack 2, nor do I have much time available to me at the moment to investigate much further so I'd be happy if someone else wanted to pick this up.


** UPDATE **
Using the following config for Rollup allows import { add } from 'ramda' to work, though it produces the same output as importing from the relative file path (i.e. including every Ramda function in the output).

import resolve from 'rollup-plugin-node-resolve';
export default {
  plugins: [resolve({ jsnext: true })]
};

@burabure
Copy link

@borisirota I love you! babel-plugin-ramda is just what I needed!
icecream

@dalgard
Copy link

dalgard commented Sep 8, 2016

Any news on this?

I guess it wouldn't be difficult to restructure Ramda to allow for this, but I don't have time myself.

@xaviervia
Copy link

xaviervia commented Oct 11, 2016

From what I see in the code, what could be done is:

  • Add a make task that, for each file directly in src, builds the same file in dist (src/map -> dist/map)
  • Setup scripts.prepublish task in package.json to cp dist/* .

Sure, the publisher will end up with a pretty dirty git state, but:

  1. Nothing that git clean can't solve
  2. If run in CI, this should not be a problem at all

Then dist and lib and so on can be added to the .npmignore to reduce the size of the package delivered to npm. I'm guessing a lot of people are requiring functions like import map from 'ramda/src/map' so maybe it's not a good idea to strip src as well.

Let me know if you like this approach and in that case, if building each file in src is doable, I can try to PR it.

@CrossEye
Copy link
Member

I think it's a good idea, and I have no problem removing src, although we might do that in a second pass, after warning about it in one release.

@MattMS
Copy link
Member

MattMS commented Oct 12, 2016

If the code is restructured, would it be worthwhile bundling the tests and code together?
So /src/pipe.js to /pipe/main.js and /test/pipe.js to /pipe/test.js.
You could include the dist files there too (/pipe/dist.js).

@CrossEye
Copy link
Member

@MattMS:

I didn't think we were discussing removing <ramdaDir>/src, but only moving those files in <ramdaDir>/dist/src directly into <ramdaDir>/dist for publication.

Your suggestion has merit of its own as well as its own controversies, but it would be a separate change, and a very separate discussion.

@polytypic
Copy link

polytypic commented May 3, 2017

Apologies for spamming here too, but there seemed to be a couple of threads discussing related issues.

I made a quick script hack that converts Ramda to support Rollup and UglifyJS2 __PURE__ annotations: ramda-rollup-hack. Works fine in my test project and reduces bundle size: roughly half of Ramda is dead code eliminated after running the script. You could also adapt the script to Ramda's build to publish Ramda with Rollup support.

Note that compared to previous attempts at similar hacks ( ping @scott-christopher ), the /*#__PURE__*/ annotations inserted to the code by the script help UglifyJS2 to perform dead code elimination. IOW, with the annotations even curried functions can be eliminated as dead code. 🎉

In my particular case, looking at the bundle after uglifying it, there seems to be a lot of code related to transducers even though I'm not making any use of those. It might make sense try to reduce the impact of transducer support in Ramda (the code is quite verbose) or change it so that it could be dead code eliminated when not used.

Addition: The hack is now also available from NPM. See here.

@CrossEye
Copy link
Member

CrossEye commented May 3, 2017

Thank you very much.

I look forward to investigating this. Hopefully this evening.

@can-cc
Copy link

can-cc commented Sep 17, 2017

+1

@CrossEye
Copy link
Member

With #2254 now merged, can we close this?

@sladiri
Copy link

sladiri commented Oct 28, 2017

Sorry if this is not the right place, but I am confused about what the ES folder. I use Webpack 3 with just the Uglify plugin, and I failed to reduce my bundle with the ES exports:
import { map } from 'ramda' // Bundle with little code is about 50KB.
import { map } from 'ramda/es' // Again about 50KB.
import map from 'ramda/src/map' // Bundle reduced to about 7KB.

Thank you.

@kedashoe
Copy link
Contributor

cc @Andarist

@Andarist
Copy link
Contributor

@sladiri

Unfortunately webpack is not really that great when it comes to its tree-shaking. I hope it will be improved in the future. For some reason why I think so you might look into this.

When using webpack 2+ or rollup ramda and ramda/es are the same thing for you. The main entry point is resolved to es directory thanks to the module field in package.json.

When it comes to "cherry-picked" modules (like your ramda/src/map) they will still give you better results for webpack (they wont give any advantage in rollup though, because of the "scope hoisting" which is done by rollup - you can achieve same-ish with ModuleConcatenationPlugin for webpack). NOTE if you decide to cherry-pick ramda's modules with webpack2+ you should use es directory instead of the src, like this - import map from 'ramda/es/map'

For more information you should read this thread and its comments.

@mAAdhaTTah
Copy link

@Andarist Is there any benefit to import map from 'ramda/es/map'; vs import map from 'ramda/src/map'; from a tree-shaking perspective?

@Andarist
Copy link
Contributor

Andarist commented Nov 2, 2017

In webpack? Probably not that much in terms of tree shaking, however src is in commonjs format and es is in es modules format. es is standard javascript feature and also it is easier for static analysis.

In example webpack's ModuleConcatenationPlugin will bail out from its optimization as soon as it meets commonjs module, because it only handles es modules.

Whenever possible you should use es modules because various tooling might be optimized better for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.