ESM build #5618
ESM build #5618
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
the
CLA Signed
Any noticeable impact on bundle size? Seems like the size check action failed on permissions maybe.
This seems fine to me, if its neccessary and it works. Alternatively we could break it out into a separate script, but I dunno if I see the point. |
|
The difference seems negligible, in total it's actually a few bytes smaller? Full table❯ for fn in packages/*/dist/*.esm.js; do printf "<tr><td>%s</td><td>%s<td>%s</td></tr>\n" $(cat "${fn%.esm.js}.js" | wc -c) $(cat "$fn" | wc -c) "${fn#packages/}"; done | pbcopy
|
|
I assume we'd want to have some way to test/demo this in the monorepo. What would be the best place to do that? I think we'd need to copy over the .esm.js files to some location (playground? website? some new package?) and have an html file with an importmap to use it. Just for reference here's what some other popular JS libraries/frameworks are doing regarding esm browser builds: Vue: https://vuejs.org/guide/quick-start.html#enabling-import-maps |
| freeze: false, | ||
| interop: false, | ||
| interop: format === 'esm' ? 'esModule' : false, |
There was a problem hiding this comment.
just making a note for other reviewers: https://rollupjs.org/configuration-options/#output-interop
finally got around to reviewing this in-depth. I think it all looks good, pending some testing. I do think the playground is probably the right place to do that. Do you think we could make a PR that makes that commits the esm.js files and import map, then we could verify on the Vercel deployment? |
|
Sure, I'll look into that. I did look into adding the esm files and a static html page to the playground but it seemed surprisingly complicated to get vite to just copy static files… so I was waiting for a response to make sure it was worth diving into that rabbit hole 😆 |
|
For more context about the rabbit hole: |
|
I changed the esm fork module approach to improve compatibility - it now imports both the dev and prod modules synchronously (no dynamic imports or await) and exports symbols from one of them. By adding a sideEffect: false to the package.json it seems that (some? at least vite) bundlers understand what to do with this. I created a minimal SvelteKit project with the vanilla editor and I only got symbols from prod in the build output. |
incredible. javascript ecosystem, amirite? |
|
Hah yeah the ecosystem is pretty special, makes Python look good 🤣 |
|
I think this is in a good enough place. I have working examples for sveltekit and Astro which I think are the problem frameworks? Plus of course the vanilla esm importmap example with no bundler. |
|
Just as a follow-up sanity check I went ahead and updated my astro and svelte examples to use the released 0.14.1 and they still work. Thanks for getting this through so quickly! |
No, thank you for all the time you put into making this happen. The effort here is greatly appreciate and will definitely be a boon for the community. |
|
Hi, using this version I get the following error: error: 'import', and 'export' cannot be used outside of module code at file:///Users/Library/Caches/deno/npm/registry.npmjs.org/@lexical/react/0.14.2/LexicalErrorBoundary.esm.js:7:1 import * as modDev from './LexicalErrorBoundary.dev.esm.js'; It might be because the closest package.json does not have a type field specified, causing Deno to apply CommonJS module resolution ? |
|
I think it's likely that this is fixed in the next release by #5737 |
Lexical uses process.env under the hood to differentiate between a dev and production build. Bundling lexical as is, leaves the process.env logic in. Subsequently breaking in browser. The "bug" was introduced in facebook/lexical#5618
|
The |
|
Browser usage without a bundler works fine if you're using the correct files, you can see it working here: https://playground.lexical.dev/esm/ |
|
For what it's worth, lexical has always had these entrypoints with process.env usage. You can see them in the cjs artifacts too. You just need to pick a specific development or production conditional export. "exports": {
".": {
"import": {
"types": "./index.d.ts",
"development": "./Lexical.dev.mjs",
"production": "./Lexical.prod.mjs",
"node": "./Lexical.node.mjs",
"default": "./Lexical.mjs"
},
"require": {
"types": "./index.d.ts",
"development": "./Lexical.dev.js",
"production": "./Lexical.prod.js",
"default": "./Lexical.js"
}
}
} |
|
@etrepum Rollup,typescript,node, or some other library did not pick up the conditional imports. The workaround was easy, so no action is necessary. I'm just commenting for others to find the workaround and point out that using process.env in a browser library is odd/will lead to compatibility issues. EDIT: you can check how we used lexical in the referenced commit |
|
I know how you did it, but I'm telling you what the correct way is. process.env is only used in the fork modules, which are intended to be consumed by bundlers or environments that understand process.env. If you configure your tools to use a more specific conditional export then no compilation or other code transformations will be necessary for it to be compatible with a browser. It looks like the correct solution in your build environment would be to configure the node resolve plugin accordingly https://www.npmjs.com/package/@rollup/plugin-node-resolve#exportconditions |
This adds a parallel ESM build, in addition to the current CJS build.
Code was added to
npm run update-versionto maintain the package.json metadata for each package (declaring all of the exports & setting"sideEffect": false).The deepest rabbit hole was the prod build, which uses @ampproject/rollup-plugin-closure-compiler. I could not find an obvious issue about it but when it tries to optimize
LexicalClickableLinkPluginit ends up transformingexport default function LexicalClickableLinkPlugin(…)toexport function default(…)which is an error. I couldn't figure out how to fix it without also fixing the upstream package, but it seemed that terser was more popular for this use case so I gave that a shot and it compiled straight away.The second deepest rabbit hole was figuring out how to do a fork module for the ESM build. It seems that a lot of the ecosystem is not ready for dynamic imports and top-level await, so it currently uses the pattern of importing both the dev and prod builds and exporting one of them based on
process.env.NODE_ENV. This appears to work correctly with tree-shaking, at least with"sideEffect": falsein the package.json files using vite/esbuild.Adds an /esm/ endpoint to the playground to demonstrate an ESM native build using static html with an importmap and no bundler at all.
I did not add these to /examples/ because I have currently vendored release builds of lexical packages so they can use the esm build, but I have demonstrated that it works with SvelteKit and Astro (with some vite configuration):
The required vite configuration looks like this:
Resolves #1707.