[lexical] Fix: make the extension Flow types usable (defineExtension was uncallable) - #9132
Merged
Merged
Conversation
potatowagon
requested review from
acywatson,
etrepum,
fantactuka,
ivailop7 and
zurfyx
as code owners
September 8, 2026 04:40
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
potatowagon
force-pushed
the
flow-extension-types-callable
branch
from
September 8, 2026 07:14
2994daf to
bf77b49
Compare
…was uncallable)
The hand-maintained Flow declarations mistranslate the `unique symbol`
brands that carry an extension's Config/Output/Init types, which makes
`defineExtension` — and therefore every extension-built editor — impossible
to call from a Flow codebase.
`packages/lexical/src/extension-core/internal.ts` declares three *optional*
properties keyed by `unique symbol`:
export declare const configTypeSymbol: unique symbol;
export interface LexicalExtensionInternal<Config, Output, Init> {
readonly [configTypeSymbol]?: Config;
readonly [outputTypeSymbol]?: Output;
readonly [initTypeSymbol]?: Init;
}
The Flow mirror drops both halves of that: the symbols become plain
`symbol`, and the properties become required.
declare export const configTypeSymbol: symbol;
interface LexicalExtensionInternalConfig<Config> {
[typeof configTypeSymbol]: Config;
}
Flow has no `unique symbol`. A computed key whose type is `symbol` is an
*indexer* over all symbol keys, not a single named property, so every other
property of an extension has to conform to the indexer's value type. The
result is that no object literal can satisfy `LexicalExtension`:
Cannot call `defineExtension` with object literal bound to `extension`
because property `name` is missing in `LexicalExtensionInternal` but
exists in object literal. Any property that does not exist in
`LexicalExtensionInternal` must be compatible with its indexer
`typeof initTypeSymbol`.
Declaring all three also trips `Multiple indexers are not supported`.
There is no caller-side workaround: explicit type arguments do not help,
because the object literal itself is rejected.
This replaces the symbol-keyed brands with named optional read-only
properties, which reproduce the TypeScript semantics that matter — the three
types are carried, callers never supply the properties, and
`LexicalExtension{Config,Output,Init}` still extract them. The symbols stay
exported so the `.flow` surface keeps matching the generated `.d.ts` that
`pnpm lint-flow` compares against.
It also gives `defineExtension`'s type parameters defaults. `Config`,
`Output` and `Init` are reachable only through the optional `config`,
`build` and `init` members, so an extension declaring none of them leaves
them unconstrained; TypeScript infers silently there, Flow reports
`underconstrained-implicit-instantiation`. This surfaced only once the
indexer error was gone.
Finally, this adds `packages/lexical/flow/Lexical.flowtest.js.flow`, a
type-only consumer of the public extension API. `pnpm lint-flow` compares export
*names* between the `.flow` and `.d.ts` files, and `pnpm flow` checks the
declarations for internal consistency, but nothing in the repo ever calls
the declared API — which is why a completely uncallable `defineExtension`
passed CI. The new file closes that gap. It is type-only, is never imported
at runtime, and `flow/` is not in the package's published `files`.
The `.js.flow` extension on that file is deliberate: Flow type-checks the
contents of `.js.flow` files, while `tsc` does not recognise the extension
and both ESLint and Prettier already ignore `**/*.js.flow`. A plain `.js`
file there fails `pnpm tsc` on its Flow syntax, because tsconfig's `exclude`
only covers `./packages/*/*.js` — one level above `packages/lexical/flow/`.
Test Plan:
Verified against the identical declarations as synced into Meta's www
monorepo, using www's Flow (0.331) — the same `Lexical.js.flow` content, so
the reproduction and the fix both apply to this file.
Before: a file calling `defineExtension` produces the indexer errors quoted
above. Four spellings were tried — bare object literal, explicit type
arguments, with and without `config` — and all four fail.
After (this change, applied to that copy): `flow status` reports
`No errors!` for a consumer that
- calls `defineExtension` with no config/build/init,
- calls it with a `config` and a `build` returning an output,
- and reads both back through `LexicalExtensionConfig<typeof ext>` and
`LexicalExtensionOutput<typeof ext>`.
That consumer is what `Lexical.flowtest.js.flow` contains.
That Flow checks `.js.flow` contents (rather than treating them as
unchecked declarations) was confirmed the same way: a `.js.flow` file with a
deliberate `number = 'string'` error is reported by `flow status`.
Not verified with this repo's own `pnpm flow` (flow-bin 0.321 could not be
installed in the sandbox used), so please let CI confirm the new test file
parses cleanly under the pinned version.
potatowagon
force-pushed
the
flow-extension-types-callable
branch
from
September 8, 2026 07:25
bf77b49 to
d624e9b
Compare
zurfyx
approved these changes
Sep 8, 2026
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Sep 8, 2026
Nine commits, one conflict: `pnpm-lock.yaml`. Resolved by taking main's and re-resolving the workspace, so the only delta from main's lockfile is this branch's own additions — the `tsx` devDependency and the `lexical-compiler` and `lexical-fast-check` workspace entries. `pnpm install --frozen-lockfile` accepts the result. `Lexical.js.flow` merged cleanly with facebook#9132's extension-type fix. esbuild moved 0.27.7 to 0.28.2 under the tree-shaking tests, which measure what a bundler retains, so those were re-run first and are unaffected: 328 source modules, all retaining nothing but the three `@lexical/code-prism` modules that are meant to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
etrepum
pushed a commit
to etrepum/lexical
that referenced
this pull request
Sep 8, 2026
…romising what a compact export omits ## Description Two findings, unrelated except that both hand a caller something the value does not support. **A value was parsed once per decision, and decisions nest.** `$acceptsValue` answered "is this value in the domain" by calling `$schemaMatch`, which parses in order to return the parsed value alongside its answer — and every caller here throws that away. `objectValue` asks it of each declared field and `unionValue`'s `accepts` went through `$match`, which parses with the member it picks, so deciding cost a full parse of the subtree and then the real parse ran over it again. Ten levels of `unionValue`/`objectValue` reached **59,049** parses of one leaf. Nothing needs to parse to decide. `$acceptsValue` now asks a declared `accepts` directly, `unionValue`'s asks its members rather than `$match` (the same test `$match` applies, without the parse), and `stringValue`, `booleanValue`, `enumValue` and `rawValue` — the last schemas relying on the parse-inference — declare their domains outright. Every built-in now answers membership without parsing, so the same traversal parses each value **once**, with no cache to keep coherent. **Flow described the legacy shape for a compact call.** The compact form omits every property equal to its schema default, and `exportJSON(compact?: boolean)` returned the full type either way, so `node.exportJSON(true).text.length` type-checked against a value with no `text`. `EditorState` is extended by nothing, so `toJSON` is now overloaded there as TypeScript does it, returning `CompactSerializedEditorState` for a compact call. `exportJSON` genuinely cannot be overloaded — Flow's method-override check pairs no branch and every node from TextNode down fails to extend — so its `compact` is typed `false` rather than `boolean` on the base *and on all five subclass declarations*, which is where the lie actually lived. A compact call is refused where it is written instead of answered wrongly; a Flow caller that wants it casts to `SerializedPartial`, which is what it gets. Both are covered in `Lexical.flowtest.js.flow`, the harness facebook#9132 added for declarations that are exported, self-consistent, and still unusable. ## Test plan ### Before ``` LEAF PARSES: 59049 (20ms) # ten nested unionValue/objectValue levels, one leaf ``` ``` $ pnpm run flow # the reviewer's two expressions No errors! # node.exportJSON(true).text.length # state.toJSON(true).root.indent.toFixed() ``` ### After ``` LEAF PARSES: 1 (0ms) ``` Both expressions are now rejected: ``` Cannot call `node.exportJSON` with `true` bound to `compact` because boolean literal `true` [1] is incompatible with boolean literal `false` [2]. Cannot call `state.toJSON(...).root.indent.toFixed` because property `toFixed` is missing in undefined [1]. Found 2 errors ``` The suppression on the flowtest's negative case was checked by removing it — it reports `incompatible-type`, so the case is load-bearing rather than decorative. ``` $ pnpm run test-unit Test Files 339 passed (339) Tests 6706 passed | 1 skipped (6707) $ pnpm run generate-node-json && git status --porcelain | grep GeneratedJSON (no output — generated parsers are byte-identical) $ pnpm run ci-check No errors! ``` Parse-once is pinned as an invariant across an object, a union, each wrapper, per array element, and ten nested levels, rather than only at the depth that failed. Browser and E2E suites were not run; the change is schema-level plus Flow declarations, and no DOM or selection behavior is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merged
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The hand-maintained Flow declarations mistranslate the
unique symbolbrands that carry an extension's Config/Output/Init types, which makes
defineExtension— and therefore every extension-built editor — impossibleto call from a Flow codebase.
packages/lexical/src/extension-core/internal.tsdeclares three optionalproperties keyed by
unique symbol:The Flow mirror drops both halves of that: the symbols become plain
symbol, and the properties become required.Flow has no
unique symbol. A computed key whose type issymbolis anindexer over all symbol keys, not a single named property, so every other
property of an extension has to conform to the indexer's value type. The
result is that no object literal can satisfy
LexicalExtension:Declaring all three also trips
Multiple indexers are not supported.There is no caller-side workaround: explicit type arguments do not help,
because the object literal itself is rejected.
This replaces the symbol-keyed brands with named optional read-only
properties, which reproduce the TypeScript semantics that matter — the three
types are carried, callers never supply the properties, and
LexicalExtension{Config,Output,Init}still extract them. The symbols stayexported so the
.flowsurface keeps matching the generated.d.tsthatpnpm lint-flowcompares against.It also gives
defineExtension's type parameters defaults.Config,OutputandInitare reachable only through the optionalconfig,buildandinitmembers, so an extension declaring none of them leavesthem unconstrained; TypeScript infers silently there, Flow reports
underconstrained-implicit-instantiation. This surfaced only once theindexer error was gone.
Finally, this adds
packages/lexical/flow/Lexical.flowtest.js, a type-onlyconsumer of the public extension API.
pnpm lint-flowcompares exportnames between the
.flowand.d.tsfiles, andpnpm flowchecks thedeclarations for internal consistency, but nothing in the repo ever calls
the declared API — which is why a completely uncallable
defineExtensionpassed CI. The new file closes that gap. It is type-only, is never imported
at runtime, and
flow/is not in the package's publishedfiles.Test Plan:
Verified against the identical declarations as synced into Meta's www
monorepo, using www's Flow (0.331) — the same
Lexical.js.flowcontent, sothe reproduction and the fix both apply to this file.
Before: a file calling
defineExtensionproduces the indexer errors quotedabove. Four spellings were tried — bare object literal, explicit type
arguments, with and without
config— and all four fail.After (this change, applied to that copy):
flow statusreportsNo errors!for a consumer thatdefineExtensionwith no config/build/init,configand abuildreturning an output,LexicalExtensionConfig<typeof ext>andLexicalExtensionOutput<typeof ext>.That consumer is what
Lexical.flowtest.jscontains.Not verified with this repo's own
pnpm flow(flow-bin 0.321 could not beinstalled in the sandbox used), so please let CI confirm the new test file
parses cleanly under the pinned version.