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 think this is as far as I'm going to be able to get it today, would be great if someone else could test it to make sure it works how they expect both for existing packages and packages that had trouble building due to the lack of an esm version. I was able to stand up an astro build that works locally but I'd need more time to put it online somewhere. |
|
I did notice that I have to configure vite/esbuild explicitly in order for this configuration to work. With a skeleton sveltekit app this is what the vite.config.ts looks like: import { sveltekit } from '@sveltejs/kit/vite';
import { defineConfig } from 'vitest/config';
export default defineConfig({
plugins: [sveltekit()],
test: {
include: ['src/**/*.{test,spec}.{js,ts}']
},
ssr: {
noExternal: [/^(lexical|@lexical\/.*)$/]
},
build: {
target: 'esnext'
},
optimizeDeps: {
esbuildOptions: {
target: 'esnext'
}
}
});I also couldn't get # this does not work
npm link ~/src/lexical/packages/*/npm
# this works
for npmdir in ~/src/lexical/packages/*/npm; do pkg=$(jq -r '.name' "$npmdir/package.json"); rm -rf "./node_modules/$pkg"; cp -r "$npmdir" ."/node_modules/$pkg"; done |
This seems to solve a bunch of compatibility issues I ran into, the ecosystem just isn't ready for top-level await. With package.json marked as not having side-effects, it seems this can be optimized properly (at least by vite, tested w/ SvelteKit). I verified this by building a SvelteKit app that uses lexical and searching for strings that would only be in the dev build, e.g. `should implement "importDOM"`.
|
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 |
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.