fix: validate insert.block sibling placements against the parent array's schema#2931
Draft
christianhg wants to merge 1 commit into
Draft
fix: validate insert.block sibling placements against the parent array's schema#2931christianhg wants to merge 1 commit into
insert.block sibling placements against the parent array's schema#2931christianhg wants to merge 1 commit into
Conversation
…ray's schema
`operation.insert.block` validates position-aware: it derives a
destination from the operation's `at`, resolves the sub-schema there,
and no-ops blocks the schema rejects. But for `placement: 'before' |
'after'` the validation scope and the landing position disagreed: the
scope derived from the destination path's own sub-schema view, while
`resolveTarget` lands the block as a sibling of the enclosing block at
that destination, inside its parent array. The two only differ when
the destination points at a container member: a point at a table row
resolves toward the cell content view (which accepts text blocks),
while the sibling insertion lands in `rows` (which does not), so a
text block addressed at a row passed validation and landed as
schema-invalid data.
Sibling placements now resolve their scope from the enclosing block's
parent array: an explicit membership check against the enclosing
container's `of` decides acceptance, and the parse runs against that
array's sub-schema. The membership check is separate from the parse
because a `Schema` structurally always carries a text-block type, so
a sub-schema alone can never reject `{_type: 'block'}`, the same
reason `fitBlocksToDestination` does its own accept-list checks.
Root-level destinations keep today's behavior: the root accepts
everything the schema declares.
`tests/insert-block-placement-scope.test.tsx` pins the no-op (red
before the fix), an accepted sibling (a row beside a row), the
leaf-destination case where both derivations agree, and the
root-level sibling insert. Full editor suite (1705 chromium + 852
unit) and the plugin-table suite pass unmodified.
🦋 Changeset detectedLatest commit: 345107b The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 801.6 KB | +940 B, +0.1% |
| Internal (gzip) | 153.4 KB | +217 B, +0.1% |
| Bundled (raw) | 1.41 MB | +940 B, +0.1% |
| Bundled (gzip) | 317.6 KB | +215 B, +0.1% |
| Import time | 97ms | +0ms, +0.4% |
@portabletext/editor/behaviors
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 467 B | - |
| Internal (gzip) | 207 B | - |
| Bundled (raw) | 424 B | - |
| Bundled (gzip) | 171 B | - |
| Import time | 2ms | -0ms, -1.4% |
@portabletext/editor/plugins
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 2.7 KB | - |
| Internal (gzip) | 894 B | - |
| Bundled (raw) | 2.5 KB | - |
| Bundled (gzip) | 827 B | - |
| Import time | 7ms | +0ms, +0.2% |
@portabletext/editor/selectors
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 81.2 KB | - |
| Internal (gzip) | 14.9 KB | - |
| Bundled (raw) | 76.7 KB | - |
| Bundled (gzip) | 13.8 KB | - |
| Import time | 8ms | +0ms, +0.2% |
@portabletext/editor/traversal
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 28.0 KB | - |
| Internal (gzip) | 5.6 KB | - |
| Bundled (raw) | 27.9 KB | - |
| Bundled (gzip) | 5.5 KB | - |
| Import time | 6ms | +0ms, +0.3% |
@portabletext/editor/utils
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 29.7 KB | - |
| Internal (gzip) | 6.2 KB | - |
| Bundled (raw) | 27.1 KB | - |
| Bundled (gzip) | 5.8 KB | - |
| Import time | 6ms | +0ms, +0.1% |
🗺️ . · ./behaviors · ./plugins · ./selectors · ./traversal · ./utils · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @portabletext/markdown
Compared against main (e94e6766)
| Metric | Value | vs main (e94e676) |
|---|---|---|
| Internal (raw) | 53.8 KB | - |
| Internal (gzip) | 9.8 KB | - |
| Bundled (raw) | 348.9 KB | - |
| Bundled (gzip) | 96.3 KB | - |
| Import time | 40ms | +1ms, +3.8% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
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
1 participant
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.
While hardening table clicks in #2910 we found that
insert.blockwith{_type: 'block'}addressed at a table row's path andplacement: 'before'writes the text block intorows, schema-invalid data that nothing catches downstream. The click behaviors now guard against raising that event, but the operation itself kept writing when pointed there directly.The operation already validates position-aware: it derives a destination from the operation's
at, resolvesgetPathSubSchemathere, and no-ops blocks the schema rejects. The defect is a scope disagreement specific to sibling placements.resolveTargetlandsbefore/afterblocks as siblings of the enclosing block at the destination, inside its parent array, while the validation scope derived from the destination path's own sub-schema view. The two only differ when the destination points at a container member: a point at a table row resolves toward the cell content view, which accepts text blocks, while the sibling insertion lands inrows, which does not.Sibling placements now resolve their validation scope from the enclosing block's parent array. Acceptance is an explicit membership check against the enclosing container's
of, and the parse then runs against that array's sub-schema so fields validate in the right scope. The membership check cannot be delegated to the parse: aSchemastructurally always carries a text-block type, so a sub-schema alone can never reject{_type: 'block'}, which is the same reasonfitBlocksToDestinationlayers its own accept-list checks over the paste pipeline. Root-level destinations are untouched, the root accepts everything the schema declares, andplacement: 'auto'keeps the inner-view derivation since its landing logic (replace, merge, first-child descent) genuinely targets the destination's inside.tests/insert-block-placement-scope.test.tsxpins four corners: the disallowed sibling no-ops (red before the fix, with the block landing insiderows), an accepted sibling inserts (a row beside a row), a leaf destination where both derivations agree stays unchanged, and a root-level sibling insert beside the container still works, the contract the container escape behaviors rely on. Blast radius checked by running the full editor suite (1705 browser tests on chromium plus 852 unit tests) and the plugin-table suite, all passing unmodified.