facebook / lexical

Lexical is an extensible text editor framework that provides excellent reliability, accessibility and performance.
https://lexical.dev
MIT License
17.5k stars 1.45k forks source link

Use unnamed ESM imports for Prism modules #5828

Closed thegreatercurve closed 1 month ago

thegreatercurve commented 1 month ago

This should fix a couple of bugs which are affecting our internal sync:

Test plan

Ensure that additional Prism modules are still included in LexicalCode.mjs and LexicalCode.prod.js build by running npm run build.

Ensure that additional Prism modules are now included in WWW build file LexicalCode.prod.js by running npm run build-www.

Run an draft internal sync and ensure that everything is working correctly.

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 5, 2024 5:06pm
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 5, 2024 5:06pm
github-actions[bot] commented 1 month ago

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
packages/lexical/dist/Lexical.js 26.77 KB (0%) 536 ms (0%) 77 ms (+266.63% 🔺) 612 ms
packages/lexical-rich-text/dist/LexicalRichText.js 39.14 KB (0%) 783 ms (0%) 105 ms (+83.53% 🔺) 887 ms
packages/lexical-plain-text/dist/LexicalPlainText.js 39.11 KB (0%) 783 ms (0%) 114 ms (+79.25% 🔺) 896 ms
etrepum commented 1 month ago

For what it's worth, other bundlers may tree-shake these out entirely since they're just imports that don't look like they have side-effects. Also setting "sideEffects": true in the @lexical/code package.json might resolve that, but would need some more testing across at least vite/esbuild and webpack.

I'd be happy to help sort this out, but I don't think there's a way for me to see what haste(?) does for the internal builds.

thegreatercurve commented 1 month ago

For what it's worth, other bundlers may tree-shake these out entirely since they're just imports that don't look like they have side-effects. Also setting "sideEffects": true in the @lexical/code package.json might resolve that, but would need some more testing across at least vite/esbuild and webpack.

I'd be happy to help sort this out, but I don't think there's a way for me to see what haste(?) does for the internal builds.

@etrepum FWIW, thanks for taking a look into this, originally! Our internal build is actually handled within this repo, using the same Rollup config as the NPM build, minus a few differences using the isWWW flag in build.js (our server is called WWW).

It seems we were originally tree-shaking out the additional language modules for Prism in our Rollup config, but that's probably incorrect as we should include them internally

I'm not sure how external third-party bundlers handle unnamed ESM imports for tree-shaking, but we've always bundled the Prism plugins this way for NPM, and I don't think we've not had any reports about languages being missing from Prism, so generally, I think we should be okay to retain them.

thegreatercurve commented 1 month ago

Thanks @etrepum for confirming this still works on Astro and Next! Much appreciated.