[lexical][*] Feature: node JSON serialization schemas, with compact export - #8602
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
…gistration never uses
## Description
`afterCloneFrom` is synthesized at registration for a class that declares
schema fields, unless the class wrote its own, in which case the prototype is
left alone. The generator did not know about that second half: it emitted a
copy function for every class with fields, so five classes shipped one that was
attached to their generated code and never called. `ElementNode`, `TableNode`,
`CodeNode`, `CodeHighlightNode`, `DecoratorBlockNode`, and also `ListNode` and
`ListItemNode`, all write their own.
The generator now asks the same question registration asks. That question needs
a sharper answer than the prototype alone can give: after one registration
every class in a chain owns an `afterCloneFrom` either way, so what this module
synthesizes is marked, and `declaresOwnAfterCloneFrom` reads the marker. Both
halves of the decision now come from the same place, so a class cannot be
generated for and skipped at run time, or the reverse.
The marker is a string key on the function rather than a symbol: a `Symbol()`
call at module scope is a side effect no bundler will drop, and this module is
one every editor imports.
## Test plan
### Before
Every generated module, transformed and minified:
```
$ node genmods.mjs
min=24905 gzip=5107
```
Seven classes carried a copy function and an `afterCloneFrom` property on their
factory result, none of which registration would install.
### After
```
$ node genmods.mjs
min=24181 gzip=4675
```
724 bytes minified and 432 gzipped, from 74 lines of generated source that
could never run.
```
$ pnpm run ci-check
Running Flow...
$ tsc -p tsconfig.json
$ tsc --noEmit
Found 0 errors
```
```
$ pnpm run test-unit
Test Files 343 passed (343)
Tests 8513 passed | 1 skipped (8514)
Duration 232.76s
```
```
$ pnpm run test-integration
Test Files 7 passed (7)
Tests 613 passed (613)
Duration 521.86s
```
Clone behavior is unchanged by construction, since the generator now applies
registration's own rule; the property tests over `nodeArbitrary`, which compare
a node against its clone, are what would catch it if it were not.
E2E and browser-mode tests were not exercised: this removes generated code that
was never called, which neither suite reaches.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ns, and make a barrel re-export by star
## Description
`compileParse` returns `statements` a caller has to emit before its
expression, and Flow's `CompileParseResult` did not have them:
`compiled.statements` was a missing property and passing them to
`verifyCompiledParse` was an extra one. For a schema whose parse is bound
rather than repeated — `optional(arrayValue(numberValue()), {omitDefault:
true})` — a Flow caller could only emit the expression, which names a local
nothing declared.
Six other exports were missing outright (`CompiledExpression`, `TableNaming`,
`BindingNaming`, `NUM_BODY`, `NUM_RANGE_BODY`, `NUM_CLAMP_BODY`,
`NUM_CLAMP_HELPER_SOURCE`), and `compileParse`'s `tableBaseName` was declared
as `string` when it has taken a callback since tables were shared between
schemas. All are declared now.
**Why it drifted, twice.** The parity lint compares export *names* between a
`.js.flow` and its TypeScript module. A name it can see, a signature or a field
it cannot, so `CompileParseResult` went on matching by name while saying the
wrong thing. Two things address that:
- `LexicalCompilerSchemaJsonCodegenUsage.js.flow` uses the declarations the way
`scripts/shared/generateNodeJSON.mjs` uses them. Nothing imports it; Flow
checks it because it is under `packages/`, and ESLint skips it with every
other `.js.flow`. It is the first file in these directories that consumes a
surface rather than declaring one, which is the only way a shape is checked
at all.
- The Flow barrel now says `export *`, as `src/index.ts` does, instead of
naming all 35 exports a second time. Naming them is what let the barrel
silently stop keeping up, and the parity lint compares the barrel against
`src/index.ts` rather than against each pass, so it saw nothing wrong.
`lint-flow-types.mjs` follows `export * from` to the entry point it names,
resolving it through the same module list the entry points come from.
## Test plan
### Before
The usage fixture against the declarations as they were:
```
Cannot import `CompiledExpression` because there is no `CompiledExpression` export in
Cannot call `compileParse` because no more than 3 arguments are expected by function type [1]. [extra-arg]
Cannot call `compileParse` with function bound to `tableBaseName` because `(table: any, index: any) => string` [1] is
Cannot get `compiled.statements` because property `statements` is missing in `CompileParseResult` [1]. [prop-missing]
Cannot get `compiled.statements` because property `statements` is missing in `CompileParseResult` [1]. [prop-missing]
Cannot call `verifyCompiledParse` with object literal bound to `options` because property `statements` is extra in
Cannot get `compiled.statements` because property `statements` is missing in `CompileParseResult` [1]. [prop-missing]
Found 10 errors
```
and the parity lint, which saw only the names:
```
$ node scripts/lint-flow-types.mjs | grep -c lexical-compiler
14
```
### After
```
$ node scripts/check-flow-types.mjs
Running Flow...
Found 0 errors
$ node scripts/lint-flow-types.mjs | grep -c lexical-compiler
0
```
Every other package is untouched by the `export *` resolution, which is what
says it resolved rather than skipped:
```
$ diff parity-before.txt parity-after.txt
8d7
< 14 lexical-compiler
```
```
$ pnpm run ci-check
$ eslint ./
$ tsc -p tsconfig.json
$ tsc --noEmit
Found 0 errors
```
```
$ npx vitest run --project scripts-unit
Test Files 11 passed (11)
Tests 3013 passed (3013)
```
Declarations and a build-time lint; no runtime behavior changes, so the unit,
integration, browser and E2E suites are unaffected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ding a local it can bind
## Description
`arguments` and `eval` are legal identifiers, legal property names and legal
member accesses, so a schema property named for one passed every check the
generator makes about a name's shape. The compact exporter then emitted
```ts
const arguments = node.__args;
```
which is a SyntaxError in strict mode, and every generated module is an ES
module. The class would have shipped code that cannot be parsed.
Renamed rather than refused, since the schema is valid and type-checks: the
local becomes `arguments_`, with an underscore appended until the name is free
among the ones that scope already binds, so a sibling property spelled
`arguments_` does not collide. The serialized property keeps its own name in
both directions — `json.arguments` is a member access, which any name may be —
and the `when` predicate beside it gets the same treatment, since
`node.arguments()` is a legal call whose hoisted result is not a legal binding.
Only those two names are renamed. A property named for a reserved word is
still refused, because `const class = …` is a name the emitted form cannot
carry at all rather than one it can carry differently.
Every checked-in generated module is byte-identical: no manifest class has
such a property, which is why this needed a test rather than a regeneration.
## Test plan
### Before
```
$ npx tsx repro-strict.mts
emitted: const arguments = node.__args;
parses as a module: Unexpected eval or arguments in strict mode
```
and the new tests against the unfixed generator:
```
$ npx vitest run --project scripts-unit scripts/__tests__/unit/generateNodeJSON.test.ts
× a strict-mode binding name gets a local it can bind 14ms
× a renamed local does not collide with a sibling of that name 5ms
AssertionError: expected '/** Generated from StrictNames's ser…' to contain 'const arguments_ = node.__args;'
Tests 2 failed | 16 passed (18)
```
### After
```
$ npx tsx repro-strict.mts
emitted: const arguments_ = node.__args;
parses as a module: yes
```
```
$ npx vitest run --project scripts-unit scripts/__tests__/unit/generateNodeJSON.test.ts
Test Files 1 passed (1)
Tests 18 passed (18)
```
Both tests strip the types and parse the result as strict-mode source, which is
what a check of the name alone would miss.
```
$ pnpm run generate-node-json
```
leaves every checked-in module byte-identical.
```
$ pnpm run ci-check
$ eslint ./
$ tsc -p tsconfig.json
$ tsc --noEmit
Found 0 errors
```
```
$ pnpm run test-unit
Test Files 343 passed (343)
Tests 8515 passed | 1 skipped (8516)
Duration 241.95s
```
E2E, browser-mode and integration tests were not exercised: this is build-time
code generation, and the generated output did not change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…renames
## Description
The strict-mode rename excused `arguments` and `eval` from every refusal in
`emittable`, not just the one they needed excusing from. Neither is a reserved
word, an emitted local or a parse binding, so the only check that guard
actually suppressed was `alsoBound` — the one that refuses a `when` predicate
sharing a sibling schema key's name.
A property named `arguments` gated by a predicate of the same name therefore
renamed twice to the same local:
```ts
const arguments_ = node.__args;
const arguments_ =
(arguments_ !== "") && node.arguments();
```
Every other name is refused for this, because two roles cannot share one local
whatever it is called. Renaming does not change that, so the guard is gone and
these names are held to the same rule; what keeps them out of the original
`SyntaxError` is `localFor` giving them a local to bind, which is separate.
## Test plan
### Before
```
$ npx tsx repro-collide.mts
emitted: const arguments_ = node.__args;
emitted: const arguments_ =
refused/failed: Transform failed with 1 error:
<stdin>:4:8: ERROR: The symbol "arguments_" has already been declared
```
### After
```
$ npx tsx repro-collide.mts
refused/failed: when predicate "arguments" collides with a name the generated code binds
```
The refusal names the schema that caused it, which is what it does for every
other colliding name.
The new case in `so is a name a sibling schema key already binds`, against the
unfixed generator:
```
$ npx vitest run --project scripts-unit scripts/__tests__/unit/generateNodeJSON.test.ts
× so is a name a sibling schema key already binds 6ms
AssertionError: expected [Function] to throw an error
Tests 1 failed | 17 passed (18)
```
and with it:
```
Test Files 1 passed (1)
Tests 18 passed (18)
```
A predicate named `arguments` beside no such property is still accepted and
still renamed, which the same case asserts.
```
$ pnpm run generate-node-json
```
leaves every checked-in module byte-identical.
```
$ pnpm run ci-check
$ tsc
$ tsc --noEmit
$ tsc -p tsconfig.json
Found 0 errors
```
```
$ pnpm run test-unit
Test Files 343 passed (343)
Tests 8515 passed | 1 skipped (8516)
Duration 190.33s
```
E2E, browser-mode and integration tests were not exercised: this is build-time
code generation, and the generated output did not change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…l-table] Refactor: Let the schema carry the clone for every node that declares one
## Description
A class that writes its own `afterCloneFrom` keeps it, and registration
synthesizes nothing for it — so eight classes that declare a serialization
schema were hand-copying fields the schema already describes, and getting no
generated clone for their trouble. Two more named their property's accessors
without naming the field behind them, so the schema had nothing to copy and the
method was genuinely required.
Audited every class that declares a `json` schema. Three were copying exactly
what the schema declares and now copy nothing: `ListNode`, `ListItemNode`,
`DecoratorBlockNode`, and `TableRowNode` alongside them. The rest needed the
schema told where a property is stored, which is a declaration each of them
could already make:
- `CodeHighlightNode`'s `highlightType` and `MarkNode`'s `ids` name the field on
the one side that is a plain field access, keeping the method on the other:
`setHighlightType` normalizes a falsy value to `undefined`, and `getIDs`
hands out a copy so the export does not give a caller the node's own array.
- `TableNode`'s `rowStriping`, `frozenColumnCount` and `frozenRowCount` name
their fields on the setter side. All three setters are plain field writes;
the getters stay methods because each normalizes a falsy value to
`undefined`, which is what keeps a table with no striping from writing
`rowStriping: false`.
- `TableCellNode`'s `verticalAlign` names its field on the setter side too.
`setVerticalAlign`'s `|| undefined` is a no-op over that enum, whose parsed
values are already `undefined`, `'middle'` or `'bottom'`.
Naming a field does not move the accessor guard: a subclass that overrides one
of these methods still wins, because the field only ever stands in for it.
Two classes keep a hand-written `afterCloneFrom`, both for fields maintained
outside any schema: `ElementNode` carries `__first`, `__last`, `__size` and
`__slotHost`, and `CodeNode` carries `__isSyntaxHighlightSupported`, which the
highlighter extensions set and nothing serializes. `DecoratorNode` keeps one for
`__slotHost` and declares no schema at all.
## Test plan
### Before
```
class json gen ownFields handwritten afterCloneFrom
ElementNode true true 4 true <-- blocks generated clone
MarkNode true true 0 true <-- blocks generated clone
ListNode true true 3 true <-- blocks generated clone
ListItemNode true true 2 true <-- blocks generated clone
TableNode true true 1 true <-- blocks generated clone
TableRowNode true true 1 true <-- blocks generated clone
TableCellNode true true 5 true <-- blocks generated clone
CodeNode true true 2 true <-- blocks generated clone
CodeHighlightNode true true 0 true <-- blocks generated clone
DecoratorBlockNode true true 1 true <-- blocks generated clone
```
### After
```
class json gen ownFields handwritten afterCloneFrom
ElementNode true true 4 true <-- blocks generated clone
MarkNode true true 1 false
ListNode true true 3 false
ListItemNode true true 2 false
TableNode true true 4 false
TableRowNode true true 1 false
TableCellNode true true 6 false
CodeNode true true 2 true <-- blocks generated clone
CodeHighlightNode true true 1 false
DecoratorBlockNode true true 1 false
```
63 lines of hand-written copying gone, and eight classes that had no generated
clone now have one, which is what the `ownFields` column moving says: the
schema now knows where those properties live.
```
$ pnpm run ci-check
$ tsc -p ./tsconfig.scripts.json
$ tsc -p tsconfig.json
$ tsc --noEmit
Found 0 errors
```
```
$ pnpm run test-unit
Test Files 343 passed (343)
Tests 8515 passed | 1 skipped (8516)
Duration 190.15s
```
```
$ pnpm run test-integration
Test Files 7 passed (7)
Tests 613 passed (613)
Duration 453.19s
```
Clone behavior is what changed, and it is what the property tests over
`nodeArbitrary` compare: they build a node, clone it, and require the two to
export the same JSON. The table, list and mark suites exercise the same classes
through real edits.
E2E and browser-mode tests were not exercised.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nal setters the new field declarations stand in for, and reserve the globals generated code reads
## Description
Two findings.
**A field stands in for the *conventional* accessor unless told otherwise.**
Moving `MarkNode`'s `ids` and `TableNode`'s frozen counts onto fields pointed
the subclass-override guard at `setIds`, `setFrozenColumnCount` and
`setFrozenRowCount` — none of which exist. A subclass overriding the real
`setIDs`, `setFrozenColumns` or `setFrozenRows` was bypassed on import: its
normalization simply stopped running. Nothing failed, because a field whose
conventional accessor is absent defers to nothing, which is the right rule for
a property that has no such method and the wrong one for a property whose
method is spelled differently. The three declarations now name their method,
which widens the guard rather than moving it.
**A generated local could shadow a global the emitted code reads.** A property
named `undefined` bound `const undefined = node.__label;`, making every
omission test `undefined !== undefined` — false for every value, so the compact
form silently dropped the property. One named `Array` shadowed the
`Array.isArray` that an empty-array default comparison calls. Both are legal
identifiers, legal property names and legal member accesses, so nothing about
the name's shape catches them.
The rename that already handled `arguments` and `eval` now covers every global
the generated modules read, not only the two reachable from a scope that binds
property names today: which scope reads which global is a property of the emit
templates, and getting that wrong is the mistake the list exists to prevent. A
test holds the list to the output, so a template that starts reading a new
global fails there rather than in somebody's node.
## Test plan
### Before
A subclass that normalizes through its own setter, importing:
```
$ npx tsx repro-override.mts
setIDs override applied: ["a","a","b"]
setFrozenColumns override applied: 9
```
and the emitted exporter for a property named `undefined` or `Array`:
```
const undefined = node.__label;
const Array = node.__tags;
if (undefined !== undefined && undefined !== "") {
```
### After
```
$ npx tsx repro-override.mts
setIDs override applied: ["a","b"]
setFrozenColumns override applied: 2
```
```
const undefined_ = node.__label;
const Array_ = node.__tags;
if (undefined_ !== undefined && undefined_ !== "") {
```
The four new tests against the unfixed code:
```
$ npx vitest run --project unit packages/lexical-table/.../TableFieldAccessors.test.ts packages/lexical-mark/.../MarkGeneratedJSON.test.ts
× a subclass override of either setter still decides 142ms
× a subclass override of setIDs still decides 18ms
AssertionError: expected [ 'a', 'a', 'b' ] to deeply equal [ 'a', 'b' ]
AssertionError: expected 9 to be 2 // Object.is equality
Tests 2 failed | 2 passed (4)
```
```
$ npx vitest run --project scripts-unit scripts/__tests__/unit/generateNodeJSON.test.ts
× a local never takes the name of a global the code reads 11ms
× every global the generated modules read is reserved 4ms
AssertionError: expected '/** Generated from Globals's seriali…' to contain 'const undefined_ = node.__label;'
AssertionError: expected [ 'JSON', 'Number', 'undefined', …(3) ] to deeply equal []
Tests 2 failed | 18 passed (20)
```
```
$ pnpm run generate-node-json
```
leaves every checked-in module byte-identical: no manifest property is named
for a global, and naming a method changes the guard rather than the emitted
read.
```
$ pnpm run ci-check
$ tsc --noEmit
$ tsc -p tsconfig.json
Found 0 errors
```
```
$ pnpm run test-unit
Test Files 344 passed (344)
Tests 8519 passed | 1 skipped (8520)
Duration 191.27s
```
E2E and browser-mode tests were not exercised; the integration suite runs on
the merge that follows.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…clone so a class that writes its own can call it
## Description
A class that writes its own `afterCloneFrom` is skipped by registration, so it
owns every field it declares — including the ones its schema already describes.
`ElementNode` and `CodeNode` were each hand-copying those, which is the
boilerplate the schema exists to remove, and it has to be edited again every
time a property is added.
The copy half is now emitted at module scope and exported rather than closed
over by the class's factory. It needs nothing the factory holds: a clone copies
storage, so it is field assignments and no table reads. A class that writes its
own method imports it and calls it, and writes only the part no schema
describes:
```ts
afterCloneFrom(prevNode: this): void {
super.afterCloneFrom(prevNode);
afterCloneCodeNode(this, prevNode);
this.__isSyntaxHighlightSupported = prevNode.__isSyntaxHighlightSupported;
}
```
Forgetting the call loses a field the same way forgetting to write the
assignment did, so this trades no safety for the maintenance; what it removes
is having to keep a list in two places.
`ElementNode` keeps `__format` and `__style` by hand because neither is a field
the schema names: `format` is declared through accessors, since the serialized
value is the `ElementFormatType` string rather than the number `__format`
stores, and `__style` is not serialized at all.
The factory still carries `afterCloneFrom` only where registration would
install it, so nothing about how a class without its own method gets one
changes.
## Test plan
### Before
```ts
afterCloneFrom(prevNode: this) {
super.afterCloneFrom(prevNode);
if (this.__key === prevNode.__key) { /* structure */ }
this.__indent = prevNode.__indent;
this.__format = prevNode.__format;
this.__style = prevNode.__style;
this.__dir = prevNode.__dir;
this.__textFormat = prevNode.__textFormat;
this.__textStyle = prevNode.__textStyle;
}
```
### After
```ts
afterCloneFrom(prevNode: this) {
super.afterCloneFrom(prevNode);
if (this.__key === prevNode.__key) { /* structure */ }
this.__format = prevNode.__format;
this.__style = prevNode.__style;
afterCloneElementNode(this, prevNode);
}
```
```
$ pnpm run ci-check
Running Flow...
$ tsc -p tsconfig.json
$ tsc --noEmit
Found 0 errors
```
```
$ pnpm run test-unit
Test Files 344 passed (344)
Tests 8519 passed | 1 skipped (8520)
Duration 190.30s
```
```
$ pnpm run test-integration
Test Files 7 passed (7)
Tests 613 passed (613)
Duration 386.32s
```
Clone behavior is what changed, and the property tests over `nodeArbitrary`
compare a node against its clone; every element and code-node suite exercises
the two classes through real edits.
The generated modules grow by the two functions that are now called rather than
dead (25,787 B minified against 24,912), and compress smaller than before
(4,745 B against 4,863) because the hand-written copies they replace are gone
from the sources.
E2E and browser-mode tests were not exercised.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tch a table reference literally
## Description
Two ways a lookup table and a schema key could collide in the generator.
**A property named for another property's table.** A table is bound in the
class's factory and read from inside the forms it encloses, so a local named
for one shadows it — and a read before that local's own declaration is a
`ReferenceError` rather than a wrong value. It takes a property spelled
`NAMEDTABLE_MODE_DECODE` beside a `mode` with a decode table. No enumeration of
the names a table *could* take decides this as exactly as the tables
themselves, since `aliasedValue` nests and numbers its tables, so the class is
now checked once both sets of names exist.
**A table reference matched as a pattern.** The filter that drops a table no
form ended up reading interpolated the name into `new RegExp(`\b${name}\b`)`.
A table's name comes from a schema key, so it may contain `$`, which reads as
an anchor: `DOLLAR_$MODE_DECODE` matched nothing, the declaration was dropped
as unreferenced while the code reading it was kept, and the emitted module
threw `ReferenceError`. It is a scan now, so nothing in a name can be read as a
pattern — and it still refuses a name that is only part of a longer identifier,
which is what the word boundaries were there for (`X_Y_ENCODE` is a prefix of
`X_Y_ENCODE_DEFAULT`).
Phase one's stubs also grew the clone helpers the previous commit exported: a
class that calls one cannot load against a stub that does not name it. The
manifest states which classes have one, since phase one cannot ask, and
generation now holds that statement to what it emitted — the same check the
factory names already get.
## Test plan
### Before
```
$ npx vitest run --project scripts-unit scripts/__tests__/unit/generateNodeJSON.test.ts
× a table reference is matched literally, not as a pattern 7ms
× a property named for a lookup table is refused 1ms
AssertionError: expected false to be true // Object.is equality
AssertionError: expected [Function] to throw an error
Tests 2 failed | 20 passed (22)
```
The first is `references('T[DOLLAR_$MODE_DECODE]', 'DOLLAR_$MODE_DECODE')`
returning false, which is the dropped declaration.
### After
```
$ npx vitest run --project scripts-unit scripts/__tests__/unit/generateNodeJSON.test.ts
Test Files 1 passed (1)
Tests 22 passed (22)
```
```
$ pnpm run generate-node-json
```
leaves every checked-in module byte-identical: no manifest property is named
for a table, and none of their names contain a pattern character.
```
$ pnpm run ci-check
$ tsc --noEmit
$ tsc -p tsconfig.json
Found 0 errors
```
```
$ pnpm run test-unit
Test Files 344 passed (344)
Tests 8521 passed | 1 skipped (8522)
Duration 190.65s
```
E2E, browser-mode and integration tests were not exercised: this is build-time
code generation, and the generated output did not change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…][lexical-rich-text][lexical-table][scripts] Refactor: Ignore what a schema setter returns ## Description Applying a serialization schema through a setter followed the setter's return. The walk kept `self = (next ?? self)` and every generated parser emitted two statements per property, `n = self.setX(...)` and `self = (n ?? self) as Klass`, with `self` and `n` declared for the purpose and a `void` setter given the meaning "unchanged". Neither path ever needed it, because both start from a writable node: `LexicalNode.updateFromJSON` reaches `$applyJSONSetters` through `$updateStateFromJSON`, which calls `getWritable()`, and `$applyImportJSON` skips that call over a DEV invariant that the node is one this update constructed. A setter's own `getWritable()` hands back the node it was called on, so the return was always the node already held. The walk's `ownField` branch has relied on exactly this all along, assigning the field with no `getWritable()` of its own. The return is now dropped on both paths. A generated parser writes every property to its `node` parameter and returns it, so `self`, `n` and the per-property cast are gone and a parser that applies only fields is no longer a different shape from one that calls a setter. `generateUpdate` loses the second emit pass it ran to choose between the two names. `SetterReturn` still admits a node or nothing, which is now a check that the name in the setter position belongs to a setter at all rather than something the walk depends on. ## Test plan A refactor with no behavior change, so there is no failing case to show before it. `pnpm run test-unit`, `pnpm run tsc`, `pnpm run flow`, `pnpm run lint` and `pnpm run prettier` all pass, and `LexicalGeneratedJSON.test.ts` confirms the checked-in modules are what the generator now writes. The E2E and browser matrices were not exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…][lexical-rich-text][lexical-table][scripts] Refactor: Rename a local the generated code cannot bind instead of refusing the class ## Description Three changes to what the generator writes and to how it decides a name. A compact exporter joined its definedness test to the property's default comparison unconditionally, so a `null` default came out as `direction !== undefined && direction !== null` — which is what `!= null` means. Every element node has one. It is now emitted as `direction != null`. A name that cannot be bound was split across two mechanisms. A reserved word and a name the templates bind (`node`, `json`, `v`) were *refused*, costing the class its generated code over what a property was called — a property named `default` failed the build — while a global was renamed. Now everything that cannot be bound is renamed the same way, with the name kept in every position where it really is a property (`json.default`, `node.__fallback`). The one collision left to refuse is a `when` predicate spelled like a sibling schema key, which renaming cannot separate. `RESERVED_GLOBALS` listed the globals the emit templates happen to read, kept honest by a test that scanned the output for them — a list a template could silently outgrow, and one whose test counted a global named in a comment. It is replaced by the globals the language itself defines: a closed set, so a template that starts calling `Object.keys` needs no edit here, and nothing in the output has to be scanned to find out. Runtime globals are deliberately not consulted, since `name in globalThis` would make the generated modules depend on the Node version that wrote them. Two smaller fixes in the same code: `checkTableLocals` was given every table the class declared rather than the ones the output keeps, so it could refuse a class over a name nothing binds; and `references()` looped forever on an empty name, which no caller passes. ## Test plan `pnpm run test-unit`, `pnpm run tsc`, `pnpm run flow`, `pnpm run lint` and `pnpm run prettier` all pass. `generateNodeJSON.test.ts` gains a case for a schema whose properties are named `default` and `node`, asserting the renamed locals, the unrenamed property keys, and that the result parses as a module; the tests that asserted the old refusals are replaced by it. The E2E and browser matrices were not exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…l-list][lexical-mark][lexical-react][lexical-rich-text][lexical-table][lexical-website][scripts] Bug Fix: Cover every generated clone helper, and make the codegen's own stub fail loudly ## Description Deriving `afterCloneFrom` from the schema removed seven hand-written methods and left nothing in their place: `CloneCarriesSchemaFields.test.ts` had a case for five classes, and none of the seven was among them. A dropped copy is silent — the field still exists on the clone, holding the constructor's default rather than the value, so the node loses data on its next `getWritable()` instead of failing anywhere. Every class the generator writes a clone helper for now has a case, plus `DecoratorBlockNode` through a concrete subclass, and `generateNodeJSON`'s own test holds the file to the manifest so adding a class to one means adding it to the other. Each case was checked by deleting the copy it covers: removing `afterCloneCodeNode(this, prevNode)` from `CodeNode`, or `node.__height = prevNode.__height` from `afterCloneTableRowNode`, fails the case it belongs to and nothing else; removing `afterCloneElementNode`'s call fails ten. The helper now places the node before applying the JSON and reads the comparison once the update has settled, which is what lets `ListItemNode` — whose `indent` is counted from its list ancestors, and whose `value` a transform renumbers — be tested at all. The phase-one stub throws instead of doing nothing. Phase one writes the stubs *in place*, so a run whose second phase fails leaves them in the tree, and `ElementNode` and `CodeNode` call their helper unconditionally: a no-op there dropped `direction`, `indent`, `textFormat` and `textStyle` from every element clone with nothing saying why. Three accessor declarations relied on the conventional `set<Prop>` name matching, which is the case `resolveSchemaField`'s DEV invariant cannot check, and none was tested: `rowStriping`, `verticalAlign` and the wrapper case the conventional-name guard exists for — `textFormat` names `getSerializedTextFormat`, which computes from the `getTextFormat` a subclass would override. All three now have a case, and each fails when the guard is removed. Also: `hoistGatedReads` took the naming function as a default over its own `reads`, which was right only because the one caller using the default passed the class's whole list — a filtered list would have named a property differently in the two exporters. It is now a required parameter. The generated clone helpers carry the same "do not edit by hand" marker as the other emitted functions. And the docs and docblocks that taught `MarkNode`'s `ids` as the in-tree example of a property with no field to copy are updated: it names one now, and no node in the tree is in that position. ## Test plan `pnpm run test-unit`, `pnpm run tsc`, `pnpm run flow`, `pnpm run lint` and `pnpm run prettier` all pass. The new coverage is 13 property-based clone cases, 3 accessor-override cases and 2 generator cases; each was verified by mutation, as described above, rather than only by passing. The E2E and browser matrices were not exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…eference to it
## Description
What a generated module declares at its top level — the numeric helpers, the
lookup tables — is decided by scanning the emitted code for each name. The
scan read string literals as code, and a schema's own values reach the output
as string literals, so a property declared `stringValue('numC(')` put `numC(`
in the module without calling anything: the helper was declared, nothing used
it, and `noUnusedLocals` failed the build with TS6133 over what a default was
spelled. The lookup-table filter runs through the same scan and had the same
hazard.
`references()` now masks the contents of every string literal before
scanning, so it answers about code alone. Double-quoted literals only, which
is every string the generator writes — values go through `JSON.stringify`,
and prettier rewrites them to single quotes only on the finished module,
after this has run. A `'` in what is scanned is therefore prose
(`ListNode's serialization schema`), and masking from one would swallow the
rest of the module.
The helpers are chosen through that scan too, rather than
`source.includes('numC(')`. Besides counting string values, a substring test
cannot tell `num(` from the `num` inside `numC(` without one, which is why
the old code had to test the longer names first and derive `num` from them.
`generatePackage` is exported for the test, like `generateUpdate` and
`generateCompactExport` before it: which declarations a module ends up with
is decided there, and no manifest class has a property whose value is spelled
like one.
## Test plan
`pnpm run test-unit`, `pnpm run tsc`, `pnpm run flow`, `pnpm run lint` and
`pnpm run prettier` all pass, and regenerating leaves every checked-in module
byte-identical. Three new cases: a class with a bounded number still gets
`numC` declared, one whose only mention of it is a string default gets
neither helper, and `references()` is held to string literals directly,
including escaped quotes and the apostrophes in the generated docblocks. Both
new cases fail if the masking is removed. The E2E and browser matrices were
not exercised.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…piled parse calls instead of scanning for them
## Description
Which numeric helpers a generated module declares was decided by scanning the
emitted parsers for each name. No scan can answer that: a schema key is a
property everywhere it appears, so `num: withField(stringValue(), {field:
'__text'})` puts `json.num` in the parser and `num:` in the exporter beside
it, and the module then declared a `num` nothing called — `noUnusedLocals`
failing the build with TS6133 over what a property was named. Masking string
literals fixed the previous spelling of this; it could not fix this one,
since a property access is code.
`compileParse` now reports the helpers its expression calls, collected the
way it already collects the tables it reads: the compile that wrote
`numC(v, 0, 1, Infinity, false)` is the thing that knows, and a wrapper or an
array reports what its inner parse needs. The generator records what each
class's parses reported and keeps it only where that class ends up with a
parser — a refused one reports helpers on the way to being refused, exactly
as it declares tables. That `numC` and `numK` call `num` is a fact about the
helper sources rather than about any expression, so it stays where the
prelude is assembled.
The table filter still scans, and still should: a table is read as
`TABLE[v]`, never as a property of one, and `checkTableLocals` already
refuses a class whose property is named for one of its tables.
## Test plan
`pnpm run test-unit`, `pnpm run tsc`, `pnpm run flow`, `pnpm run lint` and
`pnpm run prettier` all pass, and regenerating leaves every checked-in module
byte-identical. A class whose only property is called `num` now gets no
helper declaration, and that case fails if the decision goes back to scanning
the parser. `compileParse`'s report is tested directly for each numeric form
and for one buried under `optional(arrayValue(...))`, against what its
expression actually calls. The Flow declaration carries the new field and its
usage fixture exercises it. The E2E and browser matrices were not exercised.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…list][lexical-mark][lexical-playground][lexical-rich-text][lexical-table] Bug Fix: Leave a caller's default alone, and describe a schema shape by what it parses to
## Description
Two defects and two simplifications.
A default the caller hands a combinator is the caller's value, and
`makeSchema` already declines to freeze one. That is only half of it: an
enclosing schema derives its own default by parsing `undefined`, which hands
back that same object, and the recursive freeze reached straight through it.
`unionValue([...], shared)` nested in an `objectValue` left `shared`
read-only, in every build, so assigning to an object the caller still holds
threw. `transformValue` already marked what its transform returned as the
caller's; `unionValue` and `enumValue` now do the same for the default they
are handed and the members it is chosen from.
`SerializationSchemaShape<T>` pinned each field's accepted *input* to the
property type as well as its output, which rejected the combinator for the
very type it names: `numberValue()` reads a stringified number, so a
`SerializationSchemaShape<{count: number}>` would not accept
`{count: numberValue()}`. The input domain is now open; what the shape says
is what each property parses to.
`optional(nullable(x))` compiled to a test per nil returning the nil it
found. By the time the inner wrapper asks, the outer one has ruled
`undefined` out, so both branches return the value that was tested for —
`v == null ? v : …`, one comparison instead of two.
`ElementNode`'s `textFormat` and `textStyle` declared only a getter, so the
parse fell back to the conventional `setTextFormat`/`setTextStyle`. Those are
bare field writes, so the schema now names the fields, and both properties
are on the direct-field path in both directions as the comment above them
already claimed. Nothing was overriding them; every element node's generated
parser was calling a method to do an assignment.
Two tests also move off patterns the repo has better ones for:
`initializeUnitTest` for `buildEditorFromExtensions` with `using`, and a
null-check-then-cast for `$assertNodeType`.
## Test plan
`pnpm run test-unit`, `pnpm run tsc`, `pnpm run flow`, `pnpm run lint` and
`pnpm run prettier` all pass. Both defects were reproduced first — the freeze
against a `unionValue` default inside an `objectValue`, and the shape
rejection as TS2322 — and each has a case that fails if its fix is reverted.
The compiled fold and the two field declarations are checked by
`verifyCompiledParse`, which runs each emitted expression against the schema
it came from over the verification corpus. The E2E and browser matrices were
not exercised.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…][lexical-react][lexical-rich-text][lexical-table][scripts] Bug Fix: Fail the build for a manifest class the generator cannot compile, and reach `format` through its tables ## Description A schema the generator could not compile was a warning on stdout and a silent fallback: `generateUpdate` returned null and the class took the schema-driven walk, `generateCompactExport` emitted a call back into the schema for the property's omission test. That is the right answer for a class outside this repo, which gets no generated code at all. It is the wrong answer for a class in the manifest, which is a class we publish: the parser we meant to ship would simply not be there, and nothing but a line of build output would say so. Both now take a `strict` flag, `generatePackage` passes it, and a manifest class that stops compiling fails the build instead. No class in the manifest is in that position today, which is the point of checking. `ElementNode`'s `format` went through `getFormatType` and `setFormat`, and what those two do is a table lookup: the serialized value is the `ElementFormatType` string, the stored value is the flag. The schema names the tables instead, so `format` reads and writes `__format` directly in both directions, like `direction` and `indent` beside it. Both accessors are still named, because the conventional `getFormat` a field would otherwise stand in for returns the flag rather than the string — so a subclass overriding either still reclaims the property, and the read goes through `getFormatType`. The tables carry one pair neither constant has: `''`, the absence of an alignment, stored as 0. `getFormatType` spells that as `|| ''` and `setFormat` as `type !== '' ? … : 0`; a lookup has no fallback to spell it with, so it is an entry. Each is built by its own `@__NO_SIDE_EFFECTS__` function assigned to a plain `const`: `treeShakingSource` caught the destructuring form, which no bundler will drop however pure the call feeding it is. `__format` is a schema field now, so `ElementNode.afterCloneFrom` no longer copies it by hand — `afterCloneElementNode` does. ## Test plan `pnpm run test-unit`, `pnpm run tsc`, `pnpm run flow`, `pnpm run lint` and `pnpm run prettier` all pass. The strict refusals have a case each, over the two fixtures that already exercised the fallbacks. `format`'s round trip is covered by the existing serialization and property-based tests, and by `verifyCompiledParse` and `verifyTableCoversDomain`, which hold the emitted lookup to the schema's own domain. The E2E and browser matrices were not exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…][lexical-rich-text][lexical-table][lexical-website][scripts] Refactor: Name the lookup tables for the direction they serve, and let one decide membership ## Description `decode` and `encode` say nothing about which way they point, and here they point the way the words do not: `encode` maps a *serialized* value to the stored one, on the way in. Both directions are already named everywhere else in a schema — `getter` reads a property out, `setter` writes one in — so the tables take those names: | was | is | | --- | --- | | `decode` | `getterTable` | | `encode` | `setterTable` | | `decodeTableOf` | `getterTableOf` | | `encodeTableOf` | `setterTableOf` | | `encodedDefaultOf` | `setterDefaultOf` | | `X_KEY_DECODE` | `X_KEY_GETTER` | | `X_KEY_ENCODE` | `X_KEY_SETTER` | | `X_KEY_ENCODE_DEFAULT` | `X_KEY_SETTER_DEFAULT` | The generated modules are what this is for: they are checked in and read as source, and a reviewer should not have to work out which direction `ELEMENT_FORMAT_ENCODE` runs in. A parse that ends in a `setterTable` also asked its question twice. An enum compiled to a chain of `===` against its members and then a lookup that could only hit, because the members *are* the table's keys. Where that holds the lookup is the whole parse: `typeof v === 'string' && v in TABLE ? TABLE[v] : DEFAULT`, which agrees with the two-step form on every input rather than only on JSON — a non-string reaches the fallback, where the parse would have reached the schema's default and looked *that* up to the same thing, and the `typeof` is what keeps an object that stringifies to a member from being read as one. `ElementNode`'s `format` was seven string comparisons in front of a hash lookup; `TextNode`'s `mode` and `detail` were four and two. The fold is sampled against the claim itself — the table applied to `schema(v)`, over the same corpus the rest of the verification uses — rather than argued from the shape, so a schema whose table does not line up that way keeps the two-step form. ## Test plan `pnpm run test-unit`, `pnpm run tsc`, `pnpm run flow`, `pnpm run lint` and `pnpm run prettier` all pass, and the drift test confirms the checked-in modules are what the generator now writes. The E2E and browser matrices were not exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ass mangled ## Description Two things the rename in the previous commit broke, neither of which any check short of a release build could see. Every fenced code block in a renamed file lost a backtick. The pass tidied doubled backticks left by its own substitutions, and a fence is three, so ```` ```ts ```` became ``` ``ts ```. TypeScript, ESLint, Prettier and the test suite are all indifferent to the contents of a comment, so nothing failed; the docs would simply have rendered wrong. Three invariant messages gained backticks — `declares no getterTable` for `declares no getterTable table` — and an invariant message cannot contain one. `scripts/error-codes/transform-error-messages.mjs` rewrites each message into a template literal, and a backtick in the raw text is not something a template element can carry, so the release build died with Babel's `Invalid raw` naming only the file. That transform runs in `build.mjs --prod --codes` and nowhere else, which is why the whole ordinary check suite passed over it. ## Test plan `node scripts/build.mjs --prod --codes` now completes; before this it failed with `Error: packages/lexical/src/LexicalSchema.ts: Invalid raw`. `pnpm run test-unit`, `pnpm run tsc`, `pnpm run flow`, `pnpm run lint` and `pnpm run prettier` pass, as they did before — they do not reach either defect. The E2E and browser matrices were not exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
## Description `tableDecidesMembership` and the `hasOwn` beside it went in without JSDoc types. `tsconfig.scripts.json` type-checks the `.mjs` scripts through their JSDoc, so `tsc -p ./tsconfig.scripts.json` rejected them: indexing a table with `schema.defaultValue` and with `schema(value)`, both `unknown`, and two implicitly-`any` parameters. The table is keyed by the schema's own values — `verifyTableCoversDomain` is what establishes that — but only at run time, so the index is spelled for the checker at the two reads. `pnpm run ci-check` runs four separate `tsc` invocations (`tsc`, `tsc-scripts`, `tsc-extension`, `tsc-website`) and I had been running only the first, which is why this reached CI. ## Test plan `pnpm run ci-check` passes: all four `tsc` projects, Flow, Prettier and ESLint. Before this, `tsc-scripts` reported TS2538 twice and TS7006 twice in `scripts/shared/generateNodeJSON.mjs`. The E2E and browser matrices were not exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…configs loadable by Vite's native config loader ## Description Every Vite config in the repo warns that it "uses features that are unsupported by `configLoader: 'native'`, which is planned to become the default in a future major version of Vite". Two causes, both about how Node — rather than a bundler — reads these files. `scripts/vite/lexicalMonorepoPlugin.ts` and `scripts/vite/viteModuleResolution.ts` are written as ES modules, but the closest `package.json` is the repo root, which declared no `type`, so Node read them as CommonJS. The root now says `"type": "module"`. Nothing else inherits that. Every package under `packages/`, `examples/` and `dev-examples/` has a `package.json` of its own, and a manifest without a `type` is CommonJS on its own account rather than by inheritance, so none of them move. What is left is `scripts/`, `libdefs/`, `flow-typed/` and the root itself: the root has no `.js` files and its five configs are already `.mjs` or `.mts`, `scripts/` has one `.js` that is read as text by `create-www-stubs` and never imported, and the ten under `libdefs/` and `flow-typed/` are Flow declaration stubs that Node never loads. The relative imports between these files had no extension. A bundler infers one; Node does not, and the file that is actually there is a `.ts`, so that is what the specifier says now — in the playground's config, in the plugin's own import of `viteModuleResolution`, and in the other 20 configs that import either file, which would each have warned the same way when their own dev server or build ran. `allowImportingTsExtensions` is enabled at the root so TypeScript accepts those specifiers. It permits rather than requires: nothing else in the repo writes one, and `tsconfig.build.json` still emits declarations for the published packages, which is where a `.ts` specifier would matter. Unrelated to the serialization work on this branch; it is a warning the branch's CI surfaced. ## Test plan `pnpm run dev` for the playground previously printed five warnings — three extensionless imports and two "ESM syntax in a file loaded as CommonJS" — and now prints none. `vite build --mode production` in `dev-examples/shadow-dom` is likewise clean and still builds. `pnpm run ci-check` (four `tsc` projects, Flow, Prettier, ESLint), `pnpm run build-types` and `pnpm run test-unit` all pass. The E2E and browser matrices were not exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…list][lexical-mark][lexical-react][lexical-rich-text][lexical-table][scripts] Refactor: Give a generated parser one const per property instead of one reassigned scratch variable
## Description
Three changes to what a generated parser reads like, and one to the type of
the comparator a schema may carry.
Every property was parsed through one `let v: unknown`, reassigned down the
function. Each now binds a `const` named for the property, as the exporter
beside it already does, so a reader sees what is being parsed without
tracking what `v` holds at that line — and a property whose parse reads the
value exactly once binds nothing at all, because the read can simply be the
argument:
const direction = json.direction;
node.__dir = direction === null || direction === 'ltr' ? direction : null;
node.__indent = numC(json.indent, 0, 0, Infinity, true);
A collapsing `nullable`/`optional` over a kind whose parse is
`<test> ? v : <default>` wrote that parse twice — once to compare against the
default, once to return. Where the test holds the parse *is* the value, so
the two questions are one:
node.__title =
v == null || (typeof v === 'string' ? v : '') === ''
? null
: typeof v === 'string'
? v
: '';
node.__title = typeof title === 'string' && title !== '' ? title : null;
The wrapper's own nil test drops out with it, because a nil is outside the
inner domain and reaches the same branch either way. Only `string`, `boolean`
and `enum` have that shape; the test is derived from the metadata rather than
recovered from the compiled string, which cannot be done safely — an
`aliased` string ends in the same `? v : ""` its inner does.
`SerializationSchema.isEqual` declares `this: void`. Method syntax implied a
receiver the comparator is never given: every caller reads it off the schema
and calls it on its own — NodeState equality, `optional({omitDefault})` and
the compact export all do — so a comparator written to read `this.meta`
type-checked and threw. The bivariance the method syntax is for is unaffected,
since a `this` parameter is not a parameter for that purpose.
`compileParse` and `verifyCompiledParse` take the name of the variable the
expression reads from, defaulting to `v`, which is what lets the caller name
one parse per property.
## Test plan
`pnpm run ci-check`, `pnpm run test-unit` and `pnpm run build-types` pass, and
every emitted expression still goes through `verifyCompiledParse` against the
schema it came from over the verification corpus — which is what makes the
collapsing fold a checked claim rather than an argued one. A new case pins
that a comparator declaring a receiver no longer type-checks.
On the question of whether the single-assignment form helps V8: measured, and
it is a wash. Parsing a 12001-node document (`NODE_ENV=production`, so
`__DEV__` is off) runs at 21.08 hz ±0.84% against 20.65 hz ±1.47% for the
reassigned-`v` form — inside the run-to-run spread of this machine. The case
for it is that it reads better, not that it runs faster. The E2E and browser
matrices were not exercised.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…es out of the generated parser's locals
## Description
Three names the generator spells outright can be taken by a schema property,
and each one costs a well-formed class its generated parser:
- A property whose parse ends in a lookup table binds a second local,
`<local>Parsed`, for what the parse made of the value. A sibling property
really called `modeParsed` binds that name too, and two `const`s of one name
in one function body is a module that does not parse. The derived name now
goes through an allocator that knows every name the parser binds, the way
the property names themselves already do.
- The lookup spells its key `as string`, since that is what indexing an object
takes. Over a numeric domain the parsed value is `0 | 1`, which does not
overlap `string`, so TypeScript refuses the assertion outright — `TS2352`
against generated code, for a node and a schema that both type-check. The
parsed local is now annotated `unknown`, which the assertion widens rather
than converts; the lookup itself was always right, since a numeric key and
its decimal string are the same property.
- `verifyCompiledParse` runs the compiled expression against its schema in a
function taking the value under one parameter and the helpers under a second
named `SCOPE`. A property of that name compiles to an expression reading
`SCOPE`, and two parameters of one name are legal in a body that is not
strict — the second wins. The expression read the bag of helpers, disagreed
with its schema about every input, and the class fell back to the walk. The
second parameter is now named so it cannot collide with the first.
No checked-in generated module changes: every manifest class folds its table
into the membership test, so none of them emits a `Parsed` local.
Also regroups the generator's naming tests under a describe that says so —
they had accumulated under the compact form's — and drops a stale comment
about a reassigned `v` local that the per-property `const`s replaced.
## Test plan
### Before
Three new cases in `scripts/__tests__/unit/generateNodeJSON.test.ts`, against
the generator as it stood:
```
FAIL scripts/__tests__/unit/generateNodeJSON.test.ts > a lookup table declaration > a table whose keys are numbers is indexed without a refused cast
AssertionError: expected [ 'Conversion of type \'0 | 1\' to type…' ] to deeply equal []
- Expected
+ Received
+ [
+ "Conversion of type '0 | 1' to type 'string' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.",
+ ]
FAIL scripts/__tests__/unit/generateNodeJSON.test.ts > names the generated forms have to bind > nor with a sibling spelled like a local derived from a name
AssertionError: expected '/** Generated from Sibling\'s seriali…' to contain 'const modeParsed_: unknown ='
+ const mode = json.mode;
+ const modeParsed = mode === 0 || mode === 1 ? mode : 0;
+ node.__mode = (modeParsed as string) in SIBLING_MODE_SETTER ? SIBLING_MODE_SETTER[modeParsed as string] : SIBLING_MODE_SETTER_DEFAULT;
+ const modeParsed = json.modeParsed;
FAIL scripts/__tests__/unit/generateNodeJSON.test.ts > names the generated forms have to bind > a property named for the verifier’s own scope still compiles
AssertionError: the given combination of arguments (null and string) is invalid for this assertion.
❯ scripts/__tests__/unit/generateNodeJSON.test.ts:745:20
745| expect(source).toContain('const SCOPE = json.SCOPE;');
Test Files 1 failed (1)
Tests 3 failed | 26 passed (29)
```
The third is `generateUpdate` returning `null` — the class silently lost its
parser, which is the reported failure rather than an assertion about text.
### After
```
$ pnpm run test-unit
Test Files 345 passed (345)
Tests 8550 passed | 1 skipped (8551)
$ pnpm run ci-check
$ tsc --noEmit
Found 0 errors
$ tsc -p tsconfig.json
$ pnpm run generate-node-json
wrote /home/user/lexical/packages/lexical/src/LexicalGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-rich-text/src/LexicalRichTextGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-link/src/LexicalLinkGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-mark/src/LexicalMarkGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-list/src/LexicalListGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-table/src/LexicalTableGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-code-core/src/LexicalCodeCoreGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-react/src/shared/LexicalReactGeneratedJSON.ts
$ git status --short
M packages/lexical-compiler/src/SchemaJsonCodegen.ts
M scripts/__tests__/unit/generateNodeJSON.test.ts
M scripts/shared/generateNodeJSON.mjs
```
E2E is unaffected: no generated module, node class or runtime path changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…every property it binds, not from the exporter's reads
## Description
`generateUpdate` binds a local for every property in the composed schema, but
both of its name allocators were seeded from `schemaReads`, which drops a
property declared import-only (`getter: null`) because that property has no
read expression. Three consequences, all reproduced:
- A property whose parse ends in a lookup binds a second local, `<local>Parsed`.
Beside an import-only sibling really called `modeParsed`, the allocator did
not know that name was taken and handed it back unchanged — the duplicate
`const` the previous commit set out to remove.
- The rename that makes a name bindable at all — `default` to `default_` —
landed on an import-only sibling literally spelled `default_`, for the same
reason. `boundElsewhere`'s docblock claims distinct names never rename to the
same local; that was false in the import direction.
- `checkTableLocals` asks the same helper what a class binds, so an import-only
property named for a lookup table was not refused. The emitted parser then
read its table out of the serialized JSON — no build failure, a wrong value
or a TDZ `ReferenceError` per node, which is exactly what that check exists
to prevent.
`localsFor` and `localNamesOf` now work from every schema key rather than from
the reads, and `generateUpdate` seeds its derived-name allocator from the list
it is about to walk, which also drops a redundant schema walk per class.
Two smaller fixes found alongside:
- `verifyCompiledParse` hardened only the scope parameter against `valueName`.
The same body binds `num`, `numC`, `numK` and every table as `const`s, and a
`const` redeclaring a parameter is an unconditional SyntaxError — not a
`NotCompilable`, so a caller that re-throws anything else loses the build
over a name instead of naming the property. It now refuses such a
`valueName` as `NotCompilable` and allocates the scope parameter past all of
them. `verifyCompiledParse` is published API and `valueName` is a caller
option; in this repo the collision was masked only by `EMITTED_LOCALS`
living in another package.
- `fresh` was called before the branch that discards the statements binding it,
so every manifest class reserved a `<local>Parsed` name nothing emitted.
Test-side: `expectTypeChecks` filtered out file-less diagnostics, which is
where "the virtual file never compiled" is reported — the assertion could pass
having checked nothing. It now asserts the file loaded and reports those
diagnostics too. `checkableModule`'s stubs returned `unknown`, where an
assertion is unconditionally legal; they now return the real
`{readonly [key: string]: unknown}`, so the comparability check the numeric
table test is about is actually on. It also emits the numeric helper sources,
which it needs for any bounded-numeric property.
No checked-in generated module changes: every manifest class folds its table
into the membership test, so none of them binds a derived local, and no
manifest property needs a rename.
## Test plan
### Before
```
$ npx vitest run --project scripts-unit generateNodeJSON
FAIL scripts/__tests__/unit/generateNodeJSON.test.ts > names the generated forms have to bind > including a sibling the exporter never reads
AssertionError: expected '/** Generated from ImportOnly's seri…' to contain 'const default__ = json.default;'
FAIL scripts/__tests__/unit/generateNodeJSON.test.ts > names the generated forms have to bind > even when the property is one the exporter never reads
AssertionError: expected [Function] to throw an error
Test Files 1 failed (1)
Tests 2 failed | 29 passed (31)
$ npx vitest run --project unit packages/lexical-compiler/src/__tests__/unit/schemaJsonCodegen.test.ts
FAIL packages/lexical-compiler/src/__tests__/unit/schemaJsonCodegen.test.ts > verifyCompiledParse is what catches a plausible-but-wrong parse > a value named for a helper the expression calls is refused
SyntaxError {
"message": "Identifier 'num' has already been declared",
}
Test Files 1 failed (1)
Tests 1 failed | 59 passed (60)
```
The generated parser for a class with `mode` (numeric enum plus a setter
table) beside an import-only `modeParsed`, before the fix:
```ts
const mode = json.mode;
const modeParsed: unknown = mode === 0 || mode === 1 ? mode : 0;
node.__mode = (modeParsed as string) in SIB_MODE_SETTER ? ... ;
const modeParsed = json.modeParsed;
```
and `checkTableLocals` over an import-only property named for a table:
```
=== checkTableLocals ===
NO REFUSAL
```
### After
```
$ pnpm run test-unit
Test Files 345 passed (345)
Tests 8554 passed | 1 skipped (8555)
$ pnpm run ci-check
$ tsc --noEmit
Found 0 errors
$ tsc -p tsconfig.json
$ pnpm run generate-node-json && git status --short
wrote /home/user/lexical/packages/lexical/src/LexicalGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-rich-text/src/LexicalRichTextGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-link/src/LexicalLinkGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-mark/src/LexicalMarkGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-list/src/LexicalListGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-table/src/LexicalTableGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-code-core/src/LexicalCodeCoreGeneratedJSON.ts
wrote /home/user/lexical/packages/lexical-react/src/shared/LexicalReactGeneratedJSON.ts
M packages/lexical-compiler/src/SchemaJsonCodegen.ts
M packages/lexical-compiler/src/__tests__/unit/schemaJsonCodegen.test.ts
M scripts/__tests__/unit/generateNodeJSON.test.ts
M scripts/shared/generateNodeJSON.mjs
```
— no generated module changed, and the same class now emits
```ts
const mode = json.mode;
const modeParsed_: unknown = mode === 0 || mode === 1 ? mode : 0;
node.__mode = (modeParsed_ as string) in SIB_MODE_SETTER ? ... ;
const modeParsed = json.modeParsed;
```
with `checkTableLocals` reporting
```
Error: lookup table SHADOW_MODE_SETTER collides with a local the generated code binds for a property of that name
```
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
## Description
`localFor` and the `fresh` allocator in `generateUpdate` each ran their own
copy of "append an underscore until the name is free among the ones this scope
binds". The two have to agree: a property local renamed one way and a derived
local renamed another land on each other, which is the collision both of them
exist to prevent and which the last two commits fixed twice. They now share one
`freeName`, which is the only place the escape is spelled.
Behavior is unchanged in both directions — `localFor` still returns a bindable
name untouched, and `fresh` still tests its base — and `pnpm run
generate-node-json` reproduces every checked-in module byte for byte.
## Test plan
### Before
No failure to show: the duplication was correct, just stated twice. The two
loops were `scripts/shared/generateNodeJSON.mjs:581-584` and `:1557-1560`.
### After
```
$ npx vitest run --project scripts-unit --project unit generateNodeJSON schemaJsonCodegen
Test Files 2 passed (2)
Tests 91 passed (91)
$ npx tsc -p tsconfig.scripts.json
$ npx eslint scripts/shared/generateNodeJSON.mjs
$ npx prettier --check scripts/shared/generateNodeJSON.mjs
Checking formatting...
All matched files use Prettier code style!
$ pnpm run generate-node-json && git status --short
M scripts/shared/generateNodeJSON.mjs
```
— no generated module changed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…s chosen, and stop building statements the table branch discards
## Description
Four things a follow-up review found in the generator and its tests, none of
which changes a checked-in generated module:
- `generateCompactExport` declared its `isCompactDefault` parameter where the
emitted body *contained* that call's text. A schema whose string default is
spelled `isCompactDefault(` puts that text in the body without calling
anything, so the exporter grew a second parameter nothing used — the same
shape as the `numC(` scan fixed earlier in this branch. Whether any property
fell back to the run-time comparison is now recorded where that is decided.
- `writeExpression` computed the non-table statement — including an
`inlineSingleUse` scan — and then threw it away for every property with a
setter table. It is now built only on the path that uses it, as the derived
local already is.
- The setter table was pushed onto `nullPrototypeTables`, which
`verifyCompiledParse` consults only for the alias tables the compiled
expression itself reads. The push was inert, and it read as a guarantee this
code was making; the guarantee is `setterTableOf`'s, which wraps every setter
table in a null prototype at attach time. Stated as a comment instead, which
matters because the collapsed branch really does index that table with a raw
serialized value.
- `checkableModule` declared every table the class registered, where
`generatePackage` declares only the ones the emitted code reads — so a parser
was type-checked against a module carrying a getter table no parser reads. It
now applies the same `references` filter, emits `export {}` so the virtual
file is a module rather than a script (in a script a `shape` named for a
lib.dom interface merges with that interface instead of replacing it), and
uses the repo's own `lib` rather than the default for its target.
Three tests were also strengthened: the two numeric-table fixtures now each run
both checks (the module parses, and the checker accepts the lookup), and the
cases where a regression means a refusal call `generateUpdate(klass, true)`, so
a regression reports the refusal instead of failing on a `null` source.
## Test plan
### Before
```
$ npx vitest run --project scripts-unit generateNodeJSON
FAIL scripts/__tests__/unit/generateNodeJSON.test.ts > a property the compact form cannot compare as source > and a default spelled like that call is not one
AssertionError: expected '/** Generated from Spelled's seriali…' not to contain 'isCompactDefault: CompactDefaultTest,'
Test Files 1 failed (1)
Tests 1 failed | 31 passed (32)
```
### After
```
$ pnpm run test-unit
Test Files 345 passed (345)
Tests 8555 passed | 1 skipped (8556)
$ pnpm run ci-check
$ tsc --noEmit
$ tsc -p tsconfig.json
Found 0 errors
$ pnpm run generate-node-json && git status --short
M scripts/__tests__/unit/generateNodeJSON.test.ts
M scripts/shared/generateNodeJSON.mjs
```
— no generated module changed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
potatowagon
approved these changes
Sep 16, 2026
Merged
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
4 participants
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.
Description
This adds a declarative serialization schema DSL in the
jsonproperty of$config, enabling:exportJSON,updateFromJSON,importJSON, andafterCloneFrom@lexical/fast-checkpackage facilitating property tests with shrinking (and can be used for other future purposes, such as other codecs)Combinators (
stringValue,numberValue,booleanValue,enumValue,nullable,optional,arrayValue,unionValue,objectValue,aliasedValue,transformValue,rawValue,withField,withAccessors) carry a domain, a default, an equality and ametafor introspection. The schema is checked against the node at compile time, in the position it was written in: afield,getter,setterorwhenthat names nothing is an error with the correction suggested, a property that names no accessor must have a conventionalget<Prop>/set<Prop>pair that takes what it parses to and returns a node or nothing, and an accessor is named once, on the outermost combinator of a property, becausenullable,optionaland a union widen what that accessor has to accept. Development builds also check the names when the node is registered and the outermost rule when the schema is built, so a schema written without the types is held to the same contract; production builds carry the parsers and none of the checks.Three things fall out of having the declaration as data:
numberValuealso reads a number spelled as a string, andaliasedValuereads legacy spellings, so existing documents keep parsing. The baseupdateFromJSONtakesLexicalParseJSON<S>, which types each property asunknownfor that reason;LexicalUpdateJSON<T>keeps its shape, so a hand-written parser that reads typed properties from it is unaffected.@lexical/compiler'sSchemaJsonCodegenemits readable and optimized JS for the four methods at build time, checked against the schema it came from. Every node class in this repo gets all four. (A class outside this repo still takes the walk, which is the fallback the whole design rests on.) What a class's$configcarries is a factory: registration calls it with the class's composed schema, and the code reads every lookup table off that schema, so a generated module holds no copy of a table and nothing about a table's values is written at build time. A subclass inherits its parent's generated code when their compiled tables match, and runs it over its own schema. The clone is also exported on its own, so a class that requires a hand-maintainedafterCloneFromfor state that is not serialized, such asElementNode's children orCodeNode's highlighter flag, can call it rather than maintaining the boilerplate that can be generated by hand.@lexical/fast-check(new package):nodeArbitrary(klass)generates example JSON from the schema, typed as what the node accepts.Also adds a compact export:
exportJSON(true),editorState.toJSON(true),$withCompactExport. These omits properties equal to their schema default. Parsing with a total parser restores defaults by construction, compact export will round-trip with the same parsing/import code with no API changes on that side. The legacy form remains the default everywhere: it is somewhat more efficient to construct, is backwards compatible, and when compressed is roughly the same size over the wire.Closes #4104
Documentation
On this PR's preview site:
$config— the combinators, what each declares, and how a schema is checked against its node@lexical/fast-checkand property-based testsexportJSONmay serialize the instance as-is — the breaking change below, written up for usersBreaking changes
$config()must name itsextends. The runtime still infers it from the prototype chain, but the type system cannot, and it is what the composed serialization types follow from one config to the next. For a node whose superclass declares a$config()of its own —TextNode,ElementNode,LineBreakNode— omitting it is nowTS2416on the override rather than a silently truncated type. One line to add per node.{"indent": -3}on a paragraph reads as0.exportJSON()on a stale node reference writes pre-mutation values. A property the schema names as a field is read straight off the node, where each property previously went through an accessor that resolvedgetLatest(). That now includes an element'sformat, whose serialized string the schema derives from__formatthrough a lookup table rather than by callinggetFormatType(). CallgetLatest()first if you hold a reference a latergetWritable()has superseded; nothing inside Lexical does.{...super.exportJSON(), ownProps}put a subclass's properties last; they now come first, withtype/versionappended. The JSON is equivalent, butJSON.stringify(editorState.toJSON())is byte-different, so anything comparing serialized strings sees a diff.Bundle size
There is a cost for this DSL and code-generation up to ~10kB gzipped, due to the addition of the schema DSL and the generated code. Three apps in this repo, each built for production from source and tree-shaken (
vite build --mode production, terser, every JS asset, gzip -9),main(0e9a9f5a8) vs. this branch.dev-examples/shadow-domdev-examples/dom-importpackages/lexical-playgroundRoughly 6.4 kB gzipped is the floor: the schema runtime plus the generated code for the core nodes, which any editor registers. Beyond that an app pays only for the nodes it registers, and tree-shaking keeps it below the sum of the package deltas — the minimal app's packages grow by 8.5 kB gzipped between them, and the app itself grows by 6.4 kB. What shakes out is the combinators an app never calls (
unionValue,objectValue,rawValueand the union ranking behind them).The published bundles (
node scripts/build.mjs --prod --codesfrom a cleandist, terser-minified, invariant messages replaced by error codes, gzip -9).lexicalgrows by the schema runtime — the combinators, the serialization walk and the compact export — plus the generated serialization code for its own nodes; each feature package grows by the generated code for its nodes. Packages changing by under 0.1 kB are not listed.lexical@lexical/table@lexical/link@lexical/list@lexical/code-core@lexical/rich-text@lexical/mark@lexical/react@lexical/clipboardTwo packages grow that no application ships:
@lexical/compilerby 8.5 / 3.3 kB, for the codegen the build runs, and@lexical/fast-checkis new at 2.0 / 1.0 kB, which is a test dependency.Speed
Serializing and parsing a 12001-node document through the public API,
main(0e9a9f5a8) vs. this branch, measured back to back in one session (vitest bench, 8s per case after a 2s warmup, higher is better). Median of three runs per side, with the range across those runs:editorState.toJSON()editor.parseEditorState(json)Both sides run the TypeScript source with
NODE_ENV=production, so__DEV__is off and neither side pays for the development-only checks — which is what an application ships, and not whatvitest benchgives you by default. It is not the published bundle: no terser, and invariant messages are not replaced by error codes. Re-running the same three-by-three with__DEV__left on lands inside that spread on both sides (main126.2 hz / 14.4 hz, this branch 163.0 hz / 20.0 hz), so the development-only checks are not what the gap is made of — they are per-class and memoized, not per-node.The generated code is where that comes from, and
packages/lexical/src/__bench__/serialization.bench.tsholds it against the schema-driven walk it replaces. A node class that declares a schema but gets no generated code — any class outside this repo — takes the walk.Test plan
Coverage added along the way: cases for each combinator and for the composed types, including the compile-time checks (each typo, mismatch and misplaced accessor the types must refuse is a test); a codegen suite that compiles each combinator and verifies the result against the schema it came from, including generated modules type-checked with the real TypeScript checker; a drift test that regenerates every in-tree node's code in-process and fails if the checked-in output differs; and property tests over
nodeArbitraryfor round-tripping and forafterCloneFrom, one per class the generator writes a clone helper for, each verified by deleting the copy it covers. Existing serialization tests compare editor states structurally rather than by byte, for the key-order reason above.