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.
| into a paragraph, splitting the list around it when items follow. Before, | ||
| Backspace at the start of any item after the first joined it to the end of | ||
| the item above; for a top-level item that now takes a second Backspace. |
There was a problem hiding this comment.
Nested items were already outdented before #8829: @lexical/rich-text outdents an indented block when Backspace is pressed at its start, and a nested ListItemNode reports its depth as its indent. The first item after a paragraph did change: it used to join that paragraph. 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." (optional)
| a function of yours whose parameter is a `ListNode`), the return value of a function declared to return a node | ||
| subclass, or a property or array of a subclass type | ||
| (`const items: ListItemNode[] = list.getChildren()`). TypeScript reports | ||
| `Type 'LexicalNode' is not assignable to type ...` (TS2322 or TS2345) after |
There was a problem hiding this comment.
The shapes listed above report Type 'LexicalNode | null' ... or Type 'LexicalNode[]' ... (TS2322), Argument of type 'LexicalNode' ... (TS2345), and TS2740 Type 'LexicalNode' is missing the following properties from type ... for the return-value case. Consider quoting those and adding TS2740. (optional)
| Detect: calls to `getFirstChild()` without a type argument whose result goes | ||
| straight to a narrower node type, such as | ||
| `const p: ParagraphNode | null = node.getFirstChild()` or | ||
| `$styleList(root.getFirstChild())` where `$styleList` is a function of yours |
There was a problem hiding this comment.
getFirstChild() returns T | null, so this call already failed under strict before the overload change. getFirstChildOrThrow(), as in the CHANGELOG entry, shows a call that really did stop compiling. (optional)
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 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 ([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 item above. 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.