Repository navigation
[lexical-website] Documentation Update: Describe type-signature breaks in CHANGELOG.md by the call shapes they break, and add the missing v0.48 list Backspace behavior change - #9728
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
…s in CHANGELOG.md by the call shapes they break, and add the missing v0.48 list Backspace behavior change ## Description Three backfilled CHANGELOG.md entries did not tell people upgrading who is affected or what to change: - v0.46.0, #8661: the breaking entry said traversal methods "return the base node type" and only named annotated assignments. It did not say that the methods (and `$getNodeByKey` / `$getNodeByKeyOrThrow`) gained a non-generic first overload, so a call without a type argument that used to infer `T` from where its result went (a function argument, a return value, an annotated variable or array) now returns `LexicalNode` and stops compiling. Detect now names those call shapes and the TypeScript error, Migration shows the function-argument case, and the matching deprecation entry now has its own Detect and says the overloads were removed from TypeScript in v0.53.0 (#9479), while the Flow declarations keep them after #9529, instead of "Removal: not scheduled". - v0.49.0, #8877: the entry said `LexicalCommand<T>` is invariant, but Detect only named `LexicalCommand<unknown>` and explicit generics. A `registerCommand` listener whose parameter annotation is narrower or wider than the payload (for example `(event: KeyboardEvent) => ...` for `KEY_ENTER_COMMAND`, whose payload is `KeyboardEvent | null`), or a helper typed `LexicalCommand<KeyboardEvent>` called with that command, also stops compiling. Detect lists these shapes and the error, and Migration shows inferred listeners and a generic helper. - v0.48.0, #8829: not listed at all. `registerList` handles Backspace at `COMMAND_PRIORITY_BEFORE_EDITOR`, before rich text: at the start of a top-level item it turns the item into a paragraph and splits the list, where it used to join it to the item above. It is now a behavior change with a Verify line. `ListItemNode.collapseAtStart` also widened its return type from `true` to `boolean`; the entry says why that is not filed as breaking. To keep this from recurring, `.changeset/README.md` gets a "Type signature changes" section: a type change that can make existing call sites stop compiling is `[breaking]` even when it only deprecates something, and its Detect names the call shapes, including callers that rely on an inferred type argument. AGENTS.md and CONTRIBUTING.md, which already summarize the changeset format, get one paragraph each. `pnpm run check-changesets` now warns (without failing) when a `[deprecation]` or `[chore]` entry mentions a type parameter, overload, generic, return or payload type, or signature and has no `Detect:` paragraph. The pattern also covers "type params", "branded", "invariant", "narrowed return/payload" and "now returns". Neither the pending changesets nor the curated `[deprecation]` and Other entries in CHANGELOG.md trigger it. Out of scope: `packages/lexical-list/flow/LexicalList.js.flow` still declares `collapseAtStart(selection: RangeSelection): true`. Changing it is a published Flow type change that needs its own changeset. ## Changeset None needed: documentation and changeset tooling only. ## Test plan The new CHANGELOG.md wording was checked against the declarations at the v0.45.0, v0.46.0, v0.48.0 and v0.49.0 tags, and the TypeScript errors and the Migration snippets were compiled against the current sources. The list Backspace change was checked with a scratch unit test (not committed) that presses Backspace at the start of the second of three items, with and without `ListExtension`: `["list:first","paragraph:second","list:third"]` with it, and `["list:firstsecond|third"]` with rich text alone, which is what v0.47 did. ### Before With the test changes applied but not the CHANGELOG.md and `entry.mjs` changes: ``` $ pnpm vitest run scripts/__tests__/unit/changesets.test.ts scripts/__tests__/unit/changelog.test.ts × warns when a deprecation or chore changes types without Detect 4ms × parses the repository CHANGELOG.md 15ms FAIL |scripts-unit| scripts/__tests__/unit/changelog.test.ts > parseChangelog > parses the repository CHANGELOG.md AssertionError: expected { behavior: 43, breaking: 35, …(1) } to deeply equal { behavior: 44, breaking: 35, …(1) } FAIL |scripts-unit| scripts/__tests__/unit/changesets.test.ts > changeset entry format > warns when a deprecation or chore changes types without Detect AssertionError: expected { errors: [], warnings: [] } to deeply equal { errors: [], warnings: [ …(1) ] } Test Files 2 failed (2) Tests 2 failed | 63 passed (65) ``` ### After ``` $ pnpm vitest run scripts/__tests__/unit/changesets.test.ts scripts/__tests__/unit/changelog.test.ts Test Files 2 passed (2) Tests 65 passed (65) $ pnpm run check-changesets Changeset check passed: 80 pending changeset(s) valid. ``` Prettier and ESLint pass on the changed files.
32def95 to
252996a
Compare
There was a problem hiding this comment.
This looks good. The backfilled entries are much more useful to people upgrading, and the #8661 and #8877 call shapes and Migration snippets hold up against the old and new declarations. A few optional, non-blocking suggestions, mostly wording:
CHANGELOG.md:988-990(v0.48.0 Backspace entry). Nested list items were already outdented before #8829:@lexical/rich-textoutdents an indented block when Backspace is pressed at its start ($isSelectionCollapsedAtFrontOfIndentedBlock), and a nestedListItemNodereports its depth as its indent. What changed is top-level items. That includes the first item when a paragraph comes before the list: it used to join that paragraph, and now it becomes a paragraph of its own. Suggested wording: "Before, Backspace at the start of a top-level item joined it to the end of the block above (the previous item, or a paragraph before the list); that now takes a second Backspace. Nested items were already outdented by@lexical/rich-text."CHANGELOG.md:1144and.changeset/README.md:128. The quotedType 'LexicalNode' is not assignable to type ...is not what the listed shapes report. They reportType 'LexicalNode | null' is not assignable ...orType 'LexicalNode[]' ...(TS2322),Argument of type 'LexicalNode' is not assignable ...(TS2345), and, for a return value, TS2740Type 'LexicalNode' is missing the following properties from type .... The new "Type signature changes" section asks everyDetect:to quote the error, so it would help to list TS2740 and quote the messages in a form people can search their build output for..changeset/README.md:126and:134.$styleList(root.getFirstChild())already failed understrictbefore the overload change, becausegetFirstChild<T>()returnsT | null. UsinggetFirstChildOrThrow(), as the CHANGELOG entry does, would make the example show a call that really did stop compiling.CHANGELOG.md:1254. "The Flow declarations keep them" reads as if Flow never lost the overloads. The v0.53.0 entry says they were removed from TypeScript and Flow, and #9529 brings them back in Flow in the next release.scripts/changesets/entry.mjs:110.invariantalso matches Lexical's owninvariant()helper, so a chore such as "Replace thrown errors withinvariant" is told it may be[breaking]. Dropping that alternative still catches the #8877 case throughgenericsandbranded.
Leaving the Flow collapseAtStart declaration to its own change, as the description says, makes sense.
…raversal and v0.48 list Backspace changelog entries ## Description Review follow-ups for the changelog entries this pull request rewrites: - v0.48.0, #8829: nested list items were already outdented by `@lexical/rich-text` before #8829 (a nested `ListItemNode` reports its depth as its indent). The entry now says what changed: Backspace at the start of a top-level item used to join it to the block above, either the previous item or a paragraph before the list, and now turns it into a paragraph. - v0.46.0, #8661: Detect quotes the TypeScript errors each call shape produces (TS2322, TS2345, and TS2740 for a return value). - `.changeset/README.md`: the type-signature example uses `getFirstChildOrThrow()`, since `getFirstChild()` returns `T | null` and its argument case already failed under `strict`. ## Test plan The quoted errors come from compiling the four call shapes (annotated `getFirstChild()`, annotated `getChildren()`, `getFirstChildOrThrow()` as an argument, and as a return value) against the current declarations, whose non-generic overloads are the ones v0.46.0 added: ``` t.ts(5,7): error TS2322: Type 'LexicalNode | null' is not assignable to type 'ParagraphNode | null'. t.ts(6,7): error TS2322: Type 'LexicalNode[]' is not assignable to type 'ListItemNode[]'. t.ts(7,12): error TS2345: Argument of type 'LexicalNode' is not assignable to parameter of type 'ListNode'. t.ts(9,3): error TS2740: Type 'LexicalNode' is missing the following properties from type 'ListNode': __tag, __start, __listType, getTag, and 68 more. ``` A scratch unit test (not committed) pressing Backspace at the start of the first item of a list that follows a paragraph prints `["paragraph:para","paragraph:first","list:second"]` with `ListExtension` and `["paragraph:parafirst","list:second"]` with rich text alone. ### Before Same tests as the previous commit: ``` $ pnpm vitest run scripts/__tests__/unit/changesets.test.ts scripts/__tests__/unit/changelog.test.ts Test Files 2 passed (2) Tests 65 passed (65) ``` ### After ``` $ pnpm vitest run scripts/__tests__/unit/changesets.test.ts scripts/__tests__/unit/changelog.test.ts Test Files 2 passed (2) Tests 65 passed (65) $ pnpm run check-changesets Changeset check passed: 80 pending changeset(s) valid. ```
There was a problem hiding this comment.
This looks good: the backfilled entries now say who is affected and what to change, and the call shapes and Migration snippets in the #8661 and #8877 entries hold up against the released declarations. A few optional, non-blocking suggestions:
CHANGELOG.md:1258(v0.46.0 deprecation,Removal:). "The Flow declarations keep them, still deprecated, after #9529" reads as if Flow never lost the overloads. v0.53.0 removed them from Flow too (the v0.53.0 entry atCHANGELOG.md:77says "removed from TypeScript and Flow"), and the pending changeset from #9529 brings them back in the next release. Suggested: "Removal: v0.53.0 removed them from TypeScript and Flow (#9479); #9529 restores them, still deprecated, in the Flow declarations of the next release."CHANGELOG.md:1145-1149. TS2740 is not specific to return values. Any...OrThrow()result that goes to a subclass-typed variable, property or return value reports it. A nullable return reports TS2322, andgetParentreportsType 'ElementNode | null' .... All three codes are listed, so only the "for a return value" attribution is off. The.changeset/README.mdexample ("a TS2322/TS2740 error for the assignment") already puts it well.CHANGELOG.md:992-995and.changeset/README.md:97-100. The new "Type signature changes" section counts widening a return type as breaking when existing code can stop type checking.ListItemNode.collapseAtStartgoing fromtruetobooleandoes that: an override declared: truethat returnssuper.collapseAtStart(selection), orconst handled: true = item.collapseAtStart(selection), fails with TS2322 from v0.48.0. Either name that shape in the entry or narrow the README wording so the two agree.scripts/changesets/entry.mjs:110. The pattern warns on prose that is not about types:invariant(as in Lexical'sinvariant()helper), "now returns", "generic error message". It stays quiet for "narrow the parameter type" and "make the parameter required", which the README lists as breaking. Dropping bareinvariant(the #8877 wording is still caught bygenerics) and addingparam(?:eter)? types?would line it up with the README. A quiet case inscripts/__tests__/unit/changesets.test.tswould pin the result.
Leaving the Flow collapseAtStart declaration to its own change, as the description says, makes sense.
| Removal: not scheduled. | ||
| Detect: explicit type arguments on these methods, such as `getParentOrThrow<ListItemNode>()`. Calls without one are covered by the Breaking changes entry. | ||
|
|
||
| Removal: v0.53.0 for TypeScript; [#9479](https://github.com/meta/lexical/pull/9479) removes the generic overloads. The Flow declarations keep them, still deprecated, after [#9529](https://github.com/meta/lexical/pull/9529). |
There was a problem hiding this comment.
Optional: v0.53.0 removed these overloads from Flow too (the v0.53.0 entry at line 77 says "removed from TypeScript and Flow"), and #9529 brings them back in the next release, so "keep them" reads as if Flow never lost them. Suggested: "Removal: v0.53.0 removed them from TypeScript and Flow (#9479); #9529 restores them, still deprecated, in the Flow declarations of the next release."
| TypeScript reports TS2322 (`Type 'LexicalNode | null' is not assignable to | ||
| type ...` or `Type 'LexicalNode[]' is not assignable to type ...`), TS2345 | ||
| (`Argument of type 'LexicalNode' is not assignable to parameter of type | ||
| ...`), or, for a return value, TS2740 (`Type 'LexicalNode' is missing the | ||
| following properties from type ...`). Calls that pass a type argument (`getFirstChild<ParagraphNode>()`) |
There was a problem hiding this comment.
Optional: TS2740 is not specific to return values. Any ...OrThrow() result that goes to a subclass-typed variable, property or return reports it. A nullable return reports TS2322, and getParent reports Type 'ElementNode | null' .... Suggested: "... or TS2740 (Type 'LexicalNode' is missing the following properties from type ...) when an OrThrow result goes to a subclass type."
| `ListItemNode.collapseAtStart` changed to match and now returns `boolean` | ||
| instead of `true`. That widened return type is filed here rather than as | ||
| breaking because callers only test the result, and overrides that return | ||
| `true` still type check. |
There was a problem hiding this comment.
Optional: under the new "Type signature changes" rule in .changeset/README.md, this widening can break compiling code: an override declared : true that returns super.collapseAtStart(selection), or const handled: true = item.collapseAtStart(selection), fails with TS2322 from v0.48.0. Consider naming that shape here, or narrowing the README wording so the two agree.
| // Wording that suggests a change to a published type signature, which is | ||
| // breaking when existing call sites can stop compiling. | ||
| const TYPE_CHANGE_RE = | ||
| /\b(?:type[ -](?:param(?:eter)?s?|arguments?)|overloads?|generics?|branded|invariant|(?:return|payload) types?|narrow(?:ed|s)? (?:the )?(?:return|payload)|now returns|signatures?)\b/i; |
There was a problem hiding this comment.
Optional: invariant also matches Lexical's own invariant() helper, and now returns matches ordinary behavior wording, so a chore such as "Replace thrown errors in $splitNode with invariant" is told it may be [breaking]. Meanwhile "Narrow the parameter type of ..." (on the README's breaking list) passes quietly. Dropping bare invariant (the #8877 wording is still caught by generics) and adding param(?:eter)? types? would match the README more closely.
Description
Three backfilled CHANGELOG.md entries did not tell people upgrading who is affected or what to change:
$getNodeByKey/$getNodeByKeyOrThrow) gained a non-generic first overload, so a call without a type argument that used to inferTfrom where its result went (a function argument, a return value, an annotated variable or array) now returnsLexicalNodeand stops compiling. Detect now names those call shapes and quotes the TypeScript error each produces (TS2322, TS2345, TS2740), Migration shows the function-argument case, and the matching deprecation entry now has its own Detect and says the overloads were removed from TypeScript in v0.53.0 ([lexical] Breaking Change: Remove deprecated unchecked node type arguments #9479), while the Flow declarations keep them after [lexical] Bug Fix: Keep the deprecated unchecked node type arguments in the Flow declarations #9529, instead of "Removal: not scheduled".LexicalCommand<T>is invariant, but Detect only namedLexicalCommand<unknown>and explicit generics. AregisterCommandlistener whose parameter annotation is narrower or wider than the payload (for example(event: KeyboardEvent) => ...forKEY_ENTER_COMMAND, whose payload isKeyboardEvent | null), or a helper typedLexicalCommand<KeyboardEvent>called with that command, also stops compiling. Detect lists these shapes and the error, and Migration shows inferred listeners and a generic helper.registerListhandles Backspace atCOMMAND_PRIORITY_BEFORE_EDITOR, before rich text: at the start of a top-level item it turns the item into a paragraph and splits the list, where it used to join it to the block above (the previous item, or a paragraph before the list). Nested items were already outdented before [lexical-list] Bug Fix: Backspace at start of list item outdents or converts to paragraph #8829. It is now a behavior change with a Verify line.ListItemNode.collapseAtStartalso widened its return type fromtruetoboolean; the entry says why that is not filed as breaking.To keep this from recurring,
.changeset/README.mdgets a "Type signature changes" section: a type change that can make existing call sites stop compiling is[breaking]even when it only deprecates something, and its Detect names the call shapes, including callers that rely on an inferred type argument. AGENTS.md and CONTRIBUTING.md, which already summarize the changeset format, get one paragraph each.pnpm run check-changesetsnow warns (without failing) when a[deprecation]or[chore]entry mentions a type parameter, overload, generic, return or payload type, or signature and has noDetect:paragraph. The pattern also covers "type params", "branded", "invariant", "narrowed return/payload" and "now returns". Neither the pending changesets nor the curated[deprecation]and Other entries in CHANGELOG.md trigger it.Out of scope:
packages/lexical-list/flow/LexicalList.js.flowstill declarescollapseAtStart(selection: RangeSelection): true. Changing it is a published Flow type change that needs its own changeset.Changeset
None needed: documentation and changeset tooling only.
Test plan
The new CHANGELOG.md wording was checked against the declarations at the v0.45.0, v0.46.0, v0.48.0 and v0.49.0 tags, and the TypeScript errors and the Migration snippets were compiled against the current sources. The list Backspace change was checked with a scratch unit test (not committed) that presses Backspace at the start of the second of three items, with and without
ListExtension:["list:first","paragraph:second","list:third"]with it, and["list:firstsecond|third"]with rich text alone, which is what v0.47 did.Before
With the test changes applied but not the CHANGELOG.md and
entry.mjschanges:After
Prettier and ESLint pass on the changed files.