Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

Bundling Proposal #4222

Closed
DRiFTy17 opened this issue Jan 3, 2019 · 7 comments
Closed

Bundling Proposal #4222

DRiFTy17 opened this issue Jan 3, 2019 · 7 comments

Comments

@DRiFTy17
Copy link

DRiFTy17 commented Jan 3, 2019

I am curious if you guys would entertain providing the module property in the package.json files to point to the ES module index.js files? This will allow bundlers such as webpack and rollup to grab code from the ES module files where necessary instead of having to point to @material/*/index explicitly in import statements.

While we're on the subject, I was also thinking that if you were to do this, it might be worth adding a build step to target ES5? At least until the world fully moves on from having to support ES5, in which case it would be a quick change to just remove the compilation to ES5. This would remove the need to have anyone downlevel your ES6 code (in webpack for example) when they need to support ES5 for the time being.

Another option may be to just provide an ES5 build (point the module property in the package.json to that) and potentially using rollup for that as well.

Anyway, just a couple thoughts I wanted to bring up and get your opinion on. Thanks!

@kfranqueiro
Copy link
Contributor

RE the module property, see #3245 (comment).

RE build step, I'm not sure I understand the suggestion? We already include ES5 under the dist folder in every package we publish, which is referenced by the main property. Users should have the option of using our code directly (or via unpkg CDN) if they don't want to bother with their own toolchain, or if they just want to test/prototype things first.

@DRiFTy17
Copy link
Author

DRiFTy17 commented Jan 3, 2019

Sorry, I should have given a little more context.

Say I do this in a module:

import { MDCTextField } from '@material/textfield';

This will cause a bundler to pull code from the webpack UMD build you provide in the dist folder due to the main property as you mentioned. This breaks tree shaking as it cannot statically analyze imports/exports throughout the code.

My next option is to explicitly point to the ES module files (which are ES6 modules):

import { MDCTextField } from '@material/textfield/index';

Aside from having to reference /index directly, this all works great until a bundler runs on the code and now this specific module needs to be downleveled to ES5.

By providing a separate build that is still in the ES module format (using imports/exports) while also downleveling this build to ES5 we solve both issues. We can write import statements the same way and let the bundlers target the files they work best with, while still supporting ES5 and tree shaking.

As a side note, I am importing a lot of various bits and pieces from the @material/* packages. I either have to downlevel myself when importing from the ES module files, or I end up with the whole library in a production app when using the UMD bundle.

@kfranqueiro
Copy link
Contributor

kfranqueiro commented Jan 4, 2019

I have to say this seems like an unorthodox request. Consuming ES6 modules is definitely preferable for purposes of tree-shaking, but if you're consuming ES6 modules, you should be prepared to treat the contents as ES6 and build accordingly (e.g. with babel-loader in webpack).

I'm inclined to say this is not something we're likely to pursue (especially since our next priority is to convert to TypeScript, but with the intent of keeping our existing ES6 and ES5/UMD formats in the distributed packages). I'm curious though, can you show an example of a library that provides ES6 modules with contents already downleveled? Or what configuration would be necessary to accomplish this?

@DRiFTy17
Copy link
Author

DRiFTy17 commented Jan 4, 2019

There are actually quite a lot of popular packages that ship their code this way. I'm yet to run into one that doesn't actually (assuming they provide ES modules and not CommonJS).

A great example of this is the @angular/* packages such as @angular/core for example. If you look in their package you will see that the module property in the package.json is pointing to ./fesm5/core.js. This is a flattened ES module that has been downleveled to ES5 and bundled using rollup. The reason this is done is the same reason for my request. It's so that consumers of the package who still need to support older browsers do not need to add a build step to downlevel the code in each instance they are consuming it. If they didn't do this, then every instance of an Angular app would need to do extra processing during the build to ensure they are running ES5. I also construct every package I create for the web this same way. No one should have to compile my code, because I provide builds for each format they need.

The Angular team provides a great document on their package structure here.

This need will go away in the near future as we can break away from supporting older browsers (IE), in which case we will be able to just remove the need to downlevel to ES5, and maybe use ES6 as the base for a few years. ES modules were introduced in ES6, but it doesn't mean it has to be ES6 code, it can still be an ES module (using imports/exports) with the code being ES5.

Configuring this is rather easy. You can use babel to compile your current ES6 code to ES5 and it will still be an ES module.

Since you're planning to upgrade to TypeScript anyway this process will be even easier. You can just run two builds using the TypeScript compiler on your codebase, one for output to ES6, and one for output to ES5. Each build would use the "ES2015" module format, while targeting either ES6 or ES5.

An extra step that I take is to use rollup after the initial TypeScript compilation to create a flattened ES module (the same thing that Angular team does to create their fesm5 files) which combines the whole ES5 library into one file, and excludes unused code in your library with tree shaking because it's still an ES module.

@kfranqueiro
Copy link
Contributor

Thanks for the additional information. You raise a good point about this actually becoming more feasible after the TypeScript rewrite, since at that point we'd be using webpack to generate our ES6 modules either way, and choosing to downlevel it in-place should be a backwards-compatible change for anyone currently running our code through babel, because it simply shouldn't have to do anything.

I'm going to icebox this to consider in Q2 (assuming we finish TS conversion in Q1 as we're hoping to). Thanks!

@moog16
Copy link
Contributor

moog16 commented Feb 27, 2019

Stumbled upon this issue...we're actually implementing it with #4451. Look out for it :)

@kfranqueiro
Copy link
Contributor

This is resolved via #4451, which now includes ES modules with ES5 syntax and populates the module property. This will be released in 1.0.0 next week.

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

No branches or pull requests

3 participants