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

CJS and ESM for Node and Browser environments #586

Merged
merged 7 commits into from
Jul 8, 2021

Conversation

JayaKrishnaNamburu
Copy link
Member

@JayaKrishnaNamburu JayaKrishnaNamburu commented Jun 23, 2021

  • Use Interop pattern for importing and using cjs modules inside esm. Since, we are exposing module under exports too. Now build tools pick only esm` and these causes interop issues for us.
  • Most of the time, browser is the target for the packages. So, switch to adding polyfills by default instead of relying on the build tools.

#583

@JayaKrishnaNamburu
Copy link
Member Author

JayaKrishnaNamburu commented Jul 4, 2021

jss imports have been a little tricky to support the same imports in node and browsers. I made a PR here cssinjs/jss#1528

@JayaKrishnaNamburu
Copy link
Member Author

JayaKrishnaNamburu commented Jul 5, 2021

  • Don't switch to type: module yet. Instead export both cjs and esm and have .mjs for esm extension.

@JayaKrishnaNamburu JayaKrishnaNamburu changed the title Attempt to make pure esm ESM and CJS for Node and Browsers environments Jul 7, 2021
@JayaKrishnaNamburu JayaKrishnaNamburu requested a review from Utwo July 7, 2021 12:00
@JayaKrishnaNamburu JayaKrishnaNamburu changed the title ESM and CJS for Node and Browsers environments ESM and CJS for Node and Browser environments Jul 7, 2021
@codecov
Copy link

codecov bot commented Jul 7, 2021

Codecov Report

Merging #586 (6876240) into development (9bdb19c) will increase coverage by 0.02%.
The diff coverage is 56.46%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development     #586      +/-   ##
===============================================
+ Coverage        74.46%   74.48%   +0.02%     
===============================================
  Files              182      182              
  Lines             6398     6416      +18     
  Branches          1574     1579       +5     
===============================================
+ Hits              4764     4779      +15     
- Misses            1612     1615       +3     
  Partials            22       22              
Impacted Files Coverage Δ
...eport-plugin-common/src/builders/style-builders.ts 26.31% <ø> (ø)
...ugin-reactnative-component-navigation/src/index.ts 0.00% <0.00%> (ø)
...rt-plugin-reactnative-resource-loader/src/index.ts 0.00% <0.00%> (ø)
...kages/teleport-project-generator-next/src/utils.ts 86.53% <ø> (ø)
...generator/src/builder/generators/js-ast-to-code.ts 40.00% <40.00%> (-26.67%) ⬇️
...teleport-plugin-preact-base-component/src/utils.ts 66.66% <50.00%> (ø)
.../teleport-plugin-react-base-component/src/utils.ts 66.66% <50.00%> (ø)
packages/teleport-plugin-react-jss/src/utils.ts 75.55% <66.66%> (ø)
packages/teleport-code-generator/src/index.ts 91.46% <87.50%> (-0.54%) ⬇️
...leport-plugin-reactnative-app-routing/src/index.ts 97.43% <97.05%> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9bdb19c...6876240. Read the comment docs.

@JayaKrishnaNamburu
Copy link
Member Author

There are a multiple problems in switching to new style of development with esbuild mainly for monorepos.

  • In esbuild, with every supported syntax, we can transpile all the way down only to es6 many es5 features are experimental and many are not supported yet.

  • With webpack5 and es6 in node_modules. There is a bug within terser, which is not confirmed by the team yet. So it breaks in the optimisation pipeline of webpack. Which you can reproduce using https://stackblitz.com/edit/nextjs-uaz4wy?file=next.config.js. Webpack 5 along with next tends to create these broken bundles. Which works well when used with webpack4 https://stackblitz.com/edit/nextjs11-webpack4

  • When we switch a package to use type :module all the files inside are considered as esm modules. And in esm modules we always need to specify the extension along with the imports. The problem is typescript can transpile but can't bunch the files to form a build. It can do that only with systemjs and iife.

    • So, the other option to do that, is use rollup or esbuild to bundle all the files. Remember, esbuild can do till es6 and so it is out of option now. Second, using rollup, and rollup along side with rollup-plugin-typescript2 can solve this problem. But again when inside monorepos rollup can't process through symlinks.

    The next option is dropping either cjs or esm.

    Dropping cjs

  • Dropping cjs is a safe bet at the moment, since node14 can run modules natively. And browsers can run them and bundlers can use them. But one caveat, with esm is typescript. Typescript keeps complaining about import assertions with the package is not marked as module inside package.json. And with that and with the above mentioned bugs in which we can't bunch files. So, these two collide with each other.

Dropping esm

  • Riding along only with cjs actually solves a lot of things. But again now they effect browsers. Browsers can't run them natively but bundlers can use them, node can use them. So, again, we can't use it natively but you can still use via bundlers.

Using export maps

  • Yes, using export maps, we can expose both cjs and esm in node understandable way. But what lacks, is we need to name the esm file with .mjs extension. If we are not planning to switch on the type:module flag. But, typescript can't do that, so need to reply on solving the import assertions too. That's brings to square one of prolbems with esbuild and rollup with monorepos.

So, all in all there a multiple problems in moving a monorepo structure along with typescript to full esm at the moment. The root cause of all the mishap is when we want to transpile to es5 and push to npm. With es6 we are quite good and esbuild with tsc --noEmit does the job for us.

@JayaKrishnaNamburu JayaKrishnaNamburu changed the title ESM and CJS for Node and Browser environments CJS and ESM for Node and Browser environments Jul 7, 2021
@JayaKrishnaNamburu
Copy link
Member Author

@JayaKrishnaNamburu JayaKrishnaNamburu merged commit 622c3af into development Jul 8, 2021
@JayaKrishnaNamburu JayaKrishnaNamburu deleted the feat/pure-esm branch July 8, 2021 10:52
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.

1 participant