refactor(core): simplify the BlockInfo API and make it the single vocabulary for block/children plumbing - #3010
Open
nperez0111 wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
…abulary for block/children plumbing
Combines the former block-info and blockinfo-consolidation changes into
one API-simplification pass over BlockInfo.
Field renames (drop internal jargon; PM group name strings unchanged):
- bnBlock -> block
- blockContent -> content
- childContainer -> children
- isWrappedBlock -> hasContent
Precomputed derived fields, replacing hand arithmetic at dozens of
callsites (blockContent.beforePos + 1, afterPos - 1, empty-inline
checks):
- contentStart / contentEnd
- children.childrenStart / children.childrenEnd
- contentKind: "inline" | "none" | "table" | "other"
- isContentEmpty
Producer consolidation: 6 overlapping producers -> 4, named by what you
have (getBlockInfoFromNode, getBlockInfoAt, getBlockInfoNearPos,
getBlockInfoFromSelection). getBlockInfo and
getBlockInfoFromResolvedPos are deleted; getBlockInfoWithManualOffset,
getBlockInfoAtNearest and getBottomNestedBlockInfo are renamed.
One shape resolver: getBlockRegions(node) -> { outer, content?,
childrenHolder? } resolves container vs blockContainer shape in one
place, consumed by getBlockInfoFromNode. Deleted the synonym
vocabularies that answered "where do children live" in parallel:
childrenHolder.ts, ChildrenWriteTarget, fixContainer's private repair
targets, and the descend seal-variant trio (now one self-recursive
descendToLastInsertionPos returning { pos, crossedSeal }).
Deleted helpers that were bare property reads: getChildrenConfig,
isContainerType, isPlaceableAnywhere, isInsertableChild; inlined
deleteBlockCollapsingSingletonGroup and the table-caret +-4 arithmetic.
Navigation helpers (getParentBlockInfo / getPrevBlockInfo /
getNextBlockInfo / getLastDescendantBlockInfo) move from mergeBlocks.ts
to getBlockInfoFromPos.ts and become public. getParentBlockInfo now has
block-model semantics: a block inside a column parents to the column,
not the columnList, fixing the Delete-at-end climb's seal check for
container children.
nperez0111
force-pushed
the
container-blocks/blockinfo-consolidation
branch
from
August 25, 2026 14:45
488eb39 to
727a5df
Compare
nperez0111
changed the title
- Rename `insertBlocks` placements "start"/"end" to "first-child"/"last-child" (clearer about nesting; docs, tests, and jsdoc updated). - containerNav helpers now take a BlockInfo and share one shape: `descendToFirst/LastInsertionPos` both return `number | null` and accept `SealOpts` instead of the `crossedSeal` flag (callers that need "was a seal the only blocker" ask with a second seal-blind call); `getFirstLeafBlock` takes and returns BlockInfo. `ascendToInsertablePos` and `getAncestorContainers` stay position-based on purpose (their inputs are arbitrary gap positions, not blocks) and now say so in jsdoc. - Inline single-use indirections: `canMerge` + `mergeBlocks` fold into `mergeBlocksCommand` (the boolean guard makes the defensive throws statically unreachable, so they are gone); `movedNodeType` folds into `checkPlacementIsValid`; `seedRefillChildren` folds into `refillContainer`; `seedDefaultChildren` + `createContainerChildrenNode` fold into `blockToNode`; the `descend` closure folds into `getInsertionPos`. - Drop the unreachable exotic-shape guard in `getInsertionPos`: `getBlockRegions` already throws for bnBlock nodes that are neither containers nor blockContainer. - Split the inline/table-content conversion layer out of `blockToNode.ts` into `contentToNodes.ts`. - Add jsdocs to `fixContainersById` and `flattenNonInsertableBlocks`.
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.
Top of the container-blocks stack. Subsumes #3007 (now closed) — the BlockInfo simplification and the BlockInfo consolidation are one API-simplification change, so they now ship as a single layer.
This PR is all about simplifying the block-plumbing APIs: fewer names, fewer producers, no hand arithmetic, one vocabulary.
One vocabulary:
BlockInfo"Where do this block's children live?" was answered in ~8 different vocabularies (
BlockInfo.children, write-target helpers,fixContainer's private repair targets, rawbeforePos + 1walks, structural slice-walk branches, …). This PR consolidates them down to two:getBlockRegionsresolves block shape (container vsblockContainer) in one place, andBlockInfois the position-annotated view everything downstream reads.getBlockRegions(node) → { outer, content?, childrenHolder? }, consumed bygetBlockInfoFromNode. No other code asks "which shape am I".childrenHolder.ts,ChildrenWriteTarget,fixContainer's private{ blockPos, childrenStart }repair targets, and the descend seal-variant trio (now one self-recursivedescendToLastInsertionPosreturning{ pos, crossedSeal }— callers make the 1-line seal decision themselves).getChildrenConfig(=config.children),isContainerType(=config.children !== undefined),isPlaceableAnywhere(=config.placement !== "containerOnly"),isInsertableChild; inlineddeleteBlockCollapsingSingletonGroupand the table-caret±4arithmetic.Simpler
BlockInfoshapeField renames — drops the internal jargon (ProseMirror group name strings like
"bnBlock"are unchanged; only the TS API is renamed):bnBlockblockblockContentcontentchildContainerchildrenisWrappedBlockhasContentPrecomputed derived fields — callers kept re-deriving the same positions with hand arithmetic (
blockContent.beforePos + 1at 25+ sites,afterPos - 1at ~12, empty-inline-block checks at ~6).BlockInfonow carries them directly:contentStart/contentEnd— inside edges of the content nodechildren.childrenStart/children.childrenEnd— inside edges of the child containercontentKind: "inline" | "none" | "table" | "other"— replaces rawspec.contentstring checksisContentEmptyThese stay a discriminated union on
hasContent, so the existing narrowing guards keep working.Fewer producers
6 overlapping producers → 4, named by what you have; the module doc comment carries the decision table:
getBlockInfoFromNode(node, beforePos)getBlockInfoAt(doc, posBeforeBlock)getBlockInfoNearPos(source, pos)getBlockInfoFromSelection(source)getBlockInfoandgetBlockInfoFromResolvedPosare deleted;getBlockInfoWithManualOffset,getBlockInfoAtNearest, andgetBottomNestedBlockInfoare renamed (getBlockInfoFromNode,getBlockInfoNearPos,getLastDescendantBlockInfo).Navigation helpers moved & fixed
getParentBlockInfo/getPrevBlockInfo/getNextBlockInfo/getLastDescendantBlockInfomove frommergeBlocks.tstogetBlockInfoFromPos.ts(they're generic navigation, not merge logic) and become public.One parent function (deliberate behavior fix):
getParentBlockInfonow has block-model semantics — a block inside a column parents to the column, not the columnList. Fixes the Delete-at-end climb running its seal check on the wrong node for container children. Regression test added.Named positions over raw arithmetic:
childrenStart/End,contentStart/Endreplace the surviving±1/nodeSizesites; the Backspace-into-table caret now anchors on the actual previous block's content region, fixing a latent off-by-2 for nested tables.Testing
multicolumn keyboardhandlers copypaste indentation dragdrop): matches the base-branch baseline exactly — the only failure is the pre-existing webkit "Delete before shallower block" snapshot, which fails identically on the base branch head.Review round 1 (a176963)
insertBlocksplacements renamed:"start"/"end"→"first-child"/"last-child"(docs, tests, jsdoc updated; the cursor-placement"start"/"end"vocabulary ofsetTextCursorPositionis unrelated and unchanged).descendToFirst/LastInsertionPosboth returnnumber | nulland takeSealOpts(thecrossedSealreturn flag is gone — the one caller that needed "was a seal the only blocker" asks with a second seal-blind call);getFirstLeafBlocktakes and returnsBlockInfo.ascendToInsertablePos/getAncestorContainersstay position-based on purpose (their inputs are arbitrary gap positions, not blocks) and say so in jsdoc.canMerge+mergeBlocks→mergeBlocksCommand(the boolean guard makes the defensive throws statically unreachable, so they're deleted);movedNodeType→checkPlacementIsValid;seedRefillChildren→refillContainer;seedDefaultChildren+createContainerChildrenNode→blockToNode.getInsertionPosdrops its unreachable exotic-shape guard (getBlockRegionsalready throws for bnBlock nodes that are neither container norblockContainer).blockToNode.tssplit: the inline/table-content conversion layer now lives incontentToNodes.ts;blockToNode.tskeeps block assembly only.fixContainersByIdandflattenNonInsertableBlocks.