fix(richtext-lexical): block fields, custom nodes, and converters now type correctly#16919
Conversation
…s no inline blocks configured, and vice versa
📦 esbuild Bundle Analysis for payloadThis analysis was generated by esbuild-bundle-analyzer. 🤖
Largest pathsThese visualization shows top 20 largest paths in the bundle.Meta file: packages/next/meta_index.json, Out file: esbuild/index.js
Meta file: packages/payload/meta_index.json, Out file: esbuild/index.js
Meta file: packages/payload/meta_shared.json, Out file: esbuild/exports/shared.js
Meta file: packages/richtext-lexical/meta_client.json, Out file: esbuild/exports/client_optimized/index.js
Meta file: packages/ui/meta_client.json, Out file: esbuild/exports/client_optimized/index.js
Meta file: packages/ui/meta_shared.json, Out file: esbuild/exports/shared_optimized/index.js
DetailsNext to the size is how much the size has increased or decreased compared with the base branch of this PR.
|
GermanJablo
left a comment
There was a problem hiding this comment.
-
I added a commit with the regenerated types.
-
The main thing I wanted to raise is the naming strategy for the generated types. Each rich-text field generates a type that lists every node it can contain (paragraphs, headings, links, blocks, and so on). That type is named
LexicalNodes_<hash>, where<hash>is a fingerprint computed from the full contents of that list, so if any node in the editor changes, the fingerprint changes and the type gets a new name. The issue is that the type for a link's custom fields reuses that same editor fingerprint: today it takes theLexicalNodes_<hash>name and just swaps the prefix to produceLexicalLinkFields_<hash>. Because the name is tied to the fingerprint of the whole editor and not to the link fields themselves, adding an unrelated field to a block changes the editor fingerprint, which renames the link-fields type, which churns every reference to it. That is how a tiny change becomes a noisy diff. Upload fields already avoid this: they compute the hash from the upload fields' own content, so the name only changes when the upload fields change. Could we make link fields consistent with upload fields and hash them by their own content? That keeps the dedup and determinism but stops unrelated changes from cascading intoLexicalLinkFields_*renames. -
In this test, the assertion
expect(generatedTypes).not.toContain('SerializedInlineBlockNode<{blockType: string}>')reads the generated output as plain text and checks that string is absent. The problem is the generator never emits that exact string: the real output is formatted differently (with spaces and a different shape, likeSerializedInlineBlockNode<TFields extends { blockType: string }>for the declaration andSerializedInlineBlockNode<SomeBlock>for usages). Anot.toContainfor a string that can never appear always passes, so this test would stay green even if the bug came back. Could we assert against the shape the generator actually emits, so the test really guards the fix? -
SerializedBlockNode/SerializedInlineBlockNodeare written down twice: once as the real TypeScript types, and once as a hardcoded string that gets copied verbatim into every user's generatedpayload-types.ts. The two must be identical (there is even a comment saying they "MUST stay byte-for-byte in sync"), but nothing enforces it: if someone edits one and forgets the other, the type users receive in their generated file silently drifts from the real one. Could we add a small test that asserts the emitted string matches the runtime type, so this can't break unnoticed?
# Overview Fixes the `blank` and `website` template e2e suites, the only two template jobs failing in CI. ## Key Changes #### Update stale dashboard breadcrumb selector The admin e2e login helper and dashboard test waited for `span[title="Dashboard"]`, which no longer exists after the StepNav refactor in #16680. The dashboard breadcrumb now renders with class `.step-nav__first`. Applied to both `blank` and `website`. #### Fix website rich-text prop types Three website forwarder components (`Form`, `Form/Message`, `RelatedPosts`) typed their rich-text props as `DefaultTypedEditorState`. After #16919, generated fields are typed `LexicalRichText<...>`, which is not assignable to that, so the website `next build` type-check failed. They now use `SerializedEditorState`, matching the `RichText` `data` prop they forward to. ## Design Decisions `SerializedEditorState` is the type the underlying `RichText` component already accepts for its `data` prop. Typing the forwarder props the same way keeps them consistent with the sink and accepts any generated editor state, which is what other blocks (`Content`, `CallToAction`) already pass through.
The strict lexical types added by #16782 were too lossy in practice: block fields went missing, custom nodes couldn't be added to the defaults, and converters couldn't be typed. This PR fixes that.
What this fixes
Block fields show up again. When a rich text field allows multiple blocks, you can now read and write each block's own fields (e.g.
node.fields.code) - with autocomplete, and errors when you use a field that doesn't belong to that block. Before, the fields of all the blocks got merged together and each block's own fields were lost.Adding your own nodes works. Use
WithDefaultNodes<...>to combine the built-in nodes with your custom blocks. The oldDefaultNodeTypesOfhelper couldn't actually be used the way the docs showed - it produced a circular-type error. See this ts playgroundConverters can be typed. You can now pass a strongly-typed converters function to
<RichText>and spread...defaultConvertersinto it. Both used to be rejected by the type-checker.No more phantom inline-block node in generated types when an editor has no inline blocks (and the same for block nodes).
Also migrated the website template to
WithDefaultNodes, updated the converter docs, and added type + integration tests.Breaking changes
These breaking changes are relative to the 4.0 beta already on
main- not new breaking changes for users upgrading from 3.0DefaultNodeTypesOfis removed - useWithDefaultNodes.