Skip to content

Fix PropsMerge type so setNodes merge can return scalars - #6137

Merged
dylans merged 2 commits into
ianstormtaylor:mainfrom
cpruijsen:fix/issue-5490-610416c0
Oct 8, 2026
Merged

dylans merged 2 commits into
ianstormtaylor:mainfrom
cpruijsen:fix/issue-5490-610416c0

Conversation

@cpruijsen

Copy link
Copy Markdown
Contributor

description

Allow PropsMerge callbacks to return scalar property values.
packages/slate/src/interfaces/editor.ts changes the return type from object to unknown.
setNodes calls merge for each property and stores the result in Record<string, unknown>.

issue

Refs #5490.
Property-specific callback parameter types remain open, so the issue's explicitly annotated examples still fail strict function checks.

example

For a custom text node with a key property:

Transforms.setNodes(
  editor,
  { key: 'value' },
  {
    at,
    merge(existingValue, newValue) {
      if (newValue === 'value') {
        return newValue
      }
      return existingValue
    },
  }
)

Before the change, TypeScript rejects this callback because unknown is not assignable to object. The new declaration accepts the callback.

context

The shared declaration covers Transforms.setNodes and editor.setNodes and keeps the current unknown parameter contract.
Property-specific parameter and return inference must account for absent existing properties and multiple supplied keys.

packages/slate/test/props-merge.ts compiles snippets against Slate source with the existing TypeScript dependency because the Mocha Babel loader removes TypeScript annotations.

checks

  • The changed files pass ESLint and Prettier.
  • Slate source passes the TypeScript check without emit.
  • Tests pass with yarn test --watchman=false --runInBand: 1,423 Mocha tests and 97 Jest tests.
  • The regression file passes alone and fails against the merge-base source with the expected return-type assertion.
  • The change includes a patch changeset.
  • Full repository yarn lint.
  • Interactive examples and integration tests.

Tests use Node 22.17.0, which matches CI.

@changeset-bot

changeset-bot Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0a92d70

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
slate Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment thread packages/slate/test/props-merge.ts Outdated
)
`

const compileSnippet = () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • since Fix and Type Tests (and some type errors I found along the way) #6003, packages/slate/test is part of tsc --build, so yarn lint:typescript already type-checks test files in CI
  • a plain typed call in a test file covers this, no compiler harness needed
  • e.g. Transforms.setNodes(editor, {}, { merge: (prop: unknown, node: unknown) => node }) fails on main and passes with this change
  • this harness also type-checks all of slate's source every time mocha loads
  • rebase onto main (19 commits behind, no conflicts), then swap this file for the typed call

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased onto main and swapped the harness for a typed setNodes call. yarn lint:typescript fails with TS2322 on the old PropsMerge and passes with this change.

`merge` is applied to each property value and its result is stored in
`Record<string, unknown>`, so requiring an `object` return rejected
valid scalar merges (TS2322). Widen the return type to `unknown` to
match the parameters.

Refs ianstormtaylor#5490
@cpruijsen
cpruijsen force-pushed the fix/issue-5490-610416c0 branch from 187795d to 0a92d70 Compare October 8, 2026 10:05
@dylans
dylans merged commit 63134d0 into ianstormtaylor:main Oct 8, 2026
11 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants