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

ESM build #5618

Closed etrepum closed 2 months ago

etrepum commented 3 months ago

This adds a parallel ESM build, in addition to the current CJS build.

Code was added to npm run update-version to 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 LexicalClickableLinkPlugin it ends up transforming export default function LexicalClickableLinkPlugin(…) to export 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": false in the package.json files using vite/esbuild.

import * as modDev from './Lexical.dev.esm.js';
import * as modProd from './Lexical.prod.esm.js';
const mod = process.env.NODE_ENV === 'development' ? modDev : modProd;
export const $addUpdateTag = mod.$addUpdateTag;
// …

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:

import { sveltekit } from '@sveltejs/kit/vite';
import { defineConfig } from 'vitest/config';

export default defineConfig({
    plugins: [sveltekit()],
    test: {
        include: ['src/**/*.{test,spec}.{js,ts}']
    },
    // This ssr config is the part that is lexical-specific
    ssr: {
        noExternal: [/^(lexical|@lexical\/.*)$/]
    }
});

Resolves #1707.

vercel[bot] commented 3 months 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 Mar 14, 2024 0:17am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 14, 2024 0:17am
acywatson commented 2 months ago

but it seemed that terser was more popular for this use case so I gave that a shot and it compiled straight away.

Any noticeable impact on bundle size? Seems like the size check action failed on permissions maybe.

A possibly questionable move here was to do a small bit of automated refactoring of package.json files with npm run update-version to add the metadata so that bundlers can (hopefully) find the correct version to use.

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.

etrepum commented 2 months ago

The difference seems negligible, in total it's actually a few bytes smaller?

❯ for fn in packages/*/dist/*.esm.js; do printf "%s\t%s\t%s\n" $(cat "${fn%.esm.js}.js" | wc -c) $(cat "$fn" | wc -c) "$fn"; done | awk '{closure+=$1; esm+=$2;} END { print closure, esm, (100*esm/clos
ure) }'
335459 335412 99.986
Full table ```bash ❯ for fn in packages/*/dist/*.esm.js; do printf "%s%s%s\n" $(cat "${fn%.esm.js}.js" | wc -c) $(cat "$fn" | wc -c) "${fn#packages/}"; done | pbcopy ```
45004518lexical-clipboard/dist/LexicalClipboard.esm.js
1499413823lexical-code/dist/LexicalCode.esm.js
10531100lexical-dragon/dist/LexicalDragon.esm.js
11421193lexical-file/dist/LexicalFile.esm.js
894891lexical-hashtag/dist/LexicalHashtag.esm.js
541547lexical-headless/dist/LexicalHeadless.esm.js
37003662lexical-history/dist/LexicalHistory.esm.js
22662360lexical-html/dist/LexicalHtml.esm.js
46284681lexical-link/dist/LexicalLink.esm.js
1292112972lexical-list/dist/LexicalList.esm.js
30833092lexical-mark/dist/LexicalMark.esm.js
1207112228lexical-markdown/dist/LexicalMarkdown.esm.js
38744070lexical-offset/dist/LexicalOffset.esm.js
908900lexical-overflow/dist/LexicalOverflow.esm.js
49233813lexical-plain-text/dist/LexicalPlainText.esm.js
22562262lexical-react/dist/LexicalAutoEmbedPlugin.esm.js
558565lexical-react/dist/LexicalAutoFocusPlugin.esm.js
42894415lexical-react/dist/LexicalAutoLinkPlugin.esm.js
15811606lexical-react/dist/LexicalBlockWithAlignableContents.esm.js
35393697lexical-react/dist/LexicalCharacterLimitPlugin.esm.js
32523192lexical-react/dist/LexicalCheckListPlugin.esm.js
791801lexical-react/dist/LexicalClearEditorPlugin.esm.js
13711514lexical-react/dist/LexicalClickableLinkPlugin.esm.js
958955lexical-react/dist/LexicalCollaborationContext.esm.js
42854300lexical-react/dist/LexicalCollaborationPlugin.esm.js
14521502lexical-react/dist/LexicalComposer.esm.js
855855lexical-react/dist/LexicalComposerContext.esm.js
15871594lexical-react/dist/LexicalContentEditable.esm.js
66416594lexical-react/dist/LexicalContextMenuPlugin.esm.js
614604lexical-react/dist/LexicalDecoratorBlockNode.esm.js
420413lexical-react/dist/LexicalEditorRefPlugin.esm.js
19971962lexical-react/dist/LexicalErrorBoundary.esm.js
69744154lexical-react/dist/LexicalHashtagPlugin.esm.js
633624lexical-react/dist/LexicalHistoryPlugin.esm.js
18431895lexical-react/dist/LexicalHorizontalRuleNode.esm.js
737779lexical-react/dist/LexicalHorizontalRulePlugin.esm.js
12341235lexical-react/dist/LexicalLinkPlugin.esm.js
10741034lexical-react/dist/LexicalListPlugin.esm.js
847877lexical-react/dist/LexicalMarkdownShortcutPlugin.esm.js
18101850lexical-react/dist/LexicalNestedComposer.esm.js
849900lexical-react/dist/LexicalNodeEventPlugin.esm.js
65276497lexical-react/dist/LexicalNodeMenuPlugin.esm.js
795783lexical-react/dist/LexicalOnChangePlugin.esm.js
17711782lexical-react/dist/LexicalPlainTextPlugin.esm.js
17681779lexical-react/dist/LexicalRichTextPlugin.esm.js
11811260lexical-react/dist/LexicalTabIndentationPlugin.esm.js
17611883lexical-react/dist/LexicalTableOfContents.esm.js
27202770lexical-react/dist/LexicalTablePlugin.esm.js
80848253lexical-react/dist/LexicalTreeView.esm.js
82628197lexical-react/dist/LexicalTypeaheadMenuPlugin.esm.js
839843lexical-react/dist/useLexicalEditable.esm.js
695662lexical-react/dist/useLexicalIsTextContentEmpty.esm.js
935919lexical-react/dist/useLexicalNodeSelection.esm.js
715722lexical-react/dist/useLexicalSubscription.esm.js
501514lexical-react/dist/useLexicalTextEntity.esm.js
1233610903lexical-rich-text/dist/LexicalRichText.esm.js
1007410075lexical-selection/dist/LexicalSelection.esm.js
3400833302lexical-table/dist/LexicalTable.esm.js
27052778lexical-text/dist/LexicalText.esm.js
70517182lexical-utils/dist/LexicalUtils.esm.js
1907919450lexical-yjs/dist/LexicalYjs.esm.js
8970794829lexical/dist/Lexical.esm.js
etrepum commented 2 months ago

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 Preact: https://preactjs.com/guide/v10/getting-started/#using-preact-with-htm-and-importmaps three.js: https://github.com/mrdoob/three.js/blob/master/examples/webgl_animation_keyframes.html

acywatson commented 2 months ago

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 Preact: https://preactjs.com/guide/v10/getting-started/#using-preact-with-htm-and-importmaps three.js: https://github.com/mrdoob/three.js/blob/master/examples/webgl_animation_keyframes.html

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?

etrepum commented 2 months ago

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 😆

etrepum commented 2 months ago

For more context about the rabbit hole: vite.config.js can either be either commonjs or esm (based on file extension or whether package.json has "type": "module" or not). If it's commonjs it can't use packages that are only offered as esm, if it's esm it can't use require. vite-plugin-static-copy is only available in esm, and the vite config files as written today have commonjs dependencies (transform-error-messages.js is the most obvious one, did not look for others). rollup-plugin-copy works with commonjs though so I was able to use that instead.

etrepum commented 2 months ago

The vercel playground deploy should be functional in this state (at /esm/. I'll need some guidance if there's more that should be done, the last time I worked on distributing an open source JS library it was a zip file on a website about 20 years ago before there was any module standard at all 😆

etrepum commented 2 months ago

The feature that's not tested here is the esm fork modules, not sure what the best way to do that is.

etrepum commented 2 months ago

I think I've sorted out how to test the fork modules (running the release script and then doing npm link from another project) and I will need to make a few changes to build.js to address some issues - please don't merge until that's sorted.

etrepum commented 2 months ago

Still testing but this commit fixes what I've found so far

etrepum commented 2 months ago

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.

etrepum commented 2 months ago

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 npm link of the npm directories to work correctly, but copying them directly over did work. Maybe there is some special case for symlinked folders named npm?

# 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
etrepum commented 2 months ago

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.

acywatson commented 2 months ago

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?

etrepum commented 2 months ago

Hah yeah the ecosystem is pretty special, makes Python look good 🤣

etrepum commented 2 months ago

https://stackblitz.com/~/github.com/etrepum/lexical-esm-sveltekit-vanilla-js

etrepum commented 2 months ago

https://stackblitz.com/~/github.com/etrepum/lexical-esm-astro-react

etrepum commented 2 months ago

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.

etrepum commented 1 month ago

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!

acywatson commented 1 month ago

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.

nestarz commented 1 month ago

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 ?

etrepum commented 1 month ago

I think it's likely that this is fixed in the next release by #5737