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

Create .mjs entry points for module bundlers supporting ESM. #55

Merged
merged 1 commit into from Jan 22, 2019

Conversation

BenoitZugmeyer
Copy link
Contributor

In modern bundlers (ex: Webpack and Rollup), .mjs have a higher priority than .js. This change will make those load ES modules instead of the commonjs versions, so the code can be treeshaked and hoisted.

This will bypass the Babel build, but in general bundlers will use a compiler to support the ES* features needed.

In modern bundlers (ex: [Webpack][1] and [Rollup][2]), .mjs have a
higher priority than .js.  This change will make those load ES modules
instead of the commonjs versions, so the code can be [treeshaked][3] and
[hoisted][4].

This will bypass the Babel build, but in general bundlers will use a
compiler to support the ES* features needed.

[1]: https://webpack.js.org/configuration/resolve/#resolve-extensions
[2]: https://github.com/rollup/rollup-plugin-node-resolve#usage
[3]: https://webpack.js.org/guides/tree-shaking/
[4]: https://webpack.js.org/plugins/module-concatenation-plugin/
@zenparsing
Copy link
Owner

Thanks! I'll get this merged and published today.

@zenparsing zenparsing merged commit c6c8ffa into zenparsing:master Jan 22, 2019
@tyscorp
Copy link

tyscorp commented Jan 22, 2019

This change broke apollo-client for me. When upgrading I get Class constructor Observable cannot be invoked without 'new' at runtime.

@zenparsing
Copy link
Owner

Thanks for the report - reverting now.

@zenparsing
Copy link
Owner

Reverted in feb8936 / 0.8.13.

@BenoitZugmeyer
Copy link
Contributor Author

This is unfortunate, sorry for the regression. I wonder why a runtime error like this occurs when using .mjs files but not with the transpiled ones. I guess a major version bump would be needed for this change.

@zenparsing
Copy link
Owner

@BenoitZugmeyer No problem! My hypothesis is that linking to untranspiled classes caused some transpiled "subclasses" to break. I don't think it had much to do with mjs.

@jaydenseric
Copy link

Sibling .mjs files should have all the transpilations the .js ones do, except for the ESM import/export syntax. This PR just provides the untranspilled source.

Here is how I go about it for my packages:

https://unpkg.com/extract-files/

The package prepare script has separate mjs and js steps:

https://github.com/jaydenseric/extract-files/blob/v5.0.1/package.json#L58

The Babel config is identical for both, except a BABEL_ESM env var toggles modules:

https://github.com/jaydenseric/extract-files/blob/v5.0.1/babel.config.js#L7

Note that source correctly has the .mjs extension, and that the prepare:mjs script uses the Babel CLI --keep-file-extension option:

https://github.com/jaydenseric/extract-files/blob/v5.0.1/package.json#L60

That way the ESM/CJS builds have seperate .mjs and .js extensions. Using .mjs for all ESM code, even source, is pretty handy. It allows you to configure ESLint to correctly understand what files are scripts or modules, instead of assuming the whole project is one or the other:

https://github.com/jaydenseric/eslint-config-env/blob/v2.0.0/index.js#L68

But I digress. I suggest using a prepare package script instead of the build script you currently have, that way users will be able to install PRs/forks/etc. of zen-observable as a git dependency, like this:

"zen-observable": "github:jaydenseric/zen-observable#pr-branch-name-here"

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 this pull request may close these issues.

None yet

4 participants