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

Expose ES Modules (ESM) in order to allow Tree-Shaking #2204

Closed
Kmaschta opened this Issue Aug 23, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@Kmaschta
Copy link
Member

Kmaschta commented Aug 23, 2018

Currently, all the ESM import and export are transpiled to require and module.exports.

My suggestion is to not transpile these ESM imports/exports to their CommonJS counterparts.

Enabling Tree-Shaking

Any application that uses react-admin has no choice but include its whole codebase in its JS bundle.

It's the case because of the extensive usage of index.js containing a lot of imports.
This kind of files are allowing a very nice API for React Admin, but each of these are including its children modules in the bundle.

Tree shaking to the rescue ? Sadly, the Webpack Tree Shaking feature relies on these import and export to find and mark unused exports and delete them with production bundle

React Admin is already publishing its sources in src/, so what's the issue?

We indeed can import directly the ESM via thanks to:

import { Admin } from 'react-admin/src'; 

The issue is that React Admin is split in a lot of packages, and all of them relies on the transpiled versions of each others.

For example, we can import the Admin ES module from react-admin/src, but this lib imports CoreAdmin from ra-core, which is transpiled in CommonJS.
Hence, the webpack can't shake the whole tree and let a lot of unused code in the bundle.

Implementation Options & Consequences

With Breaking Change (react-admin@3 ?)

At first glance, all we need to do is change .babelrc a bit:

-    "presets": ["es2015", "stage-0", "react"],
+    "presets": [["es2015", { "modules": false }], "stage-0", "react"],

But this will introduce a major breaking change: the library users will then be forced to transpile ES modules to CommonJS on their own.

This solution would be the best in the long run, since the most common usage of React Admin is from a fresh create-react-app, as explained in the tutorial.

With An Additionnal ES Submodule

Another solution is to do like date-fns is doing in its next major version: expose the CommonJS modules by default and have a submodule where the library is transpiled with ES modules in /esm.

import { Admin } from 'react-admin;' // CommonJS
import { Admin } from 'react-admin/esm'; // ESM - Tree shaking friendly

This solution isn't easy to implement since any package should exports an ES submodule, and each parent package should duplicate some code in order to not cutting the tree.

For example, the react-admin index should import ra-core but the react-admin/esm index must import from ra-core/esm, and so on.

Maintaining A Parallel Package react-admin-esm

In a similar fashion, it is possible to fork this repository or having parallel packages.

import { Admin } from 'react-admin;' // CommonJS
import { Admin } from 'react-admin-esm'; // ESM - Tree shaking friendly

The implementation would be easier: react-admin would be a simple shell for react-admin-esm with a bit of additional transpilation.

We'll then be able to use aliases like 'react-admin': 'react-admin-esm' or something to find a way to a tree-shakable codebase.

This solution might have its drawbacks too.
In conclusion, I think there is no silver bullet to enable tree-shaking but it would be a great addition since this project is growing.

@djhi

This comment has been minimized.

Copy link
Contributor

djhi commented Aug 24, 2018

I prefer the second option: the submodule. And I agree, we should definitely do that!

@Kmaschta

This comment has been minimized.

Copy link
Member Author

Kmaschta commented Sep 4, 2018

Finally, François came with a better solution: the module key of package.json!

What that, everyone can import react-admin. The latest webpack is configured to check if there is ES modules by default 🎉

@Kmaschta

This comment has been minimized.

Copy link
Member Author

Kmaschta commented Sep 6, 2018

Yay, closed thanks to #2230 ! 💯

@Kmaschta Kmaschta closed this Sep 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment