prosemirror/prosemirror-model
1
0
Fork 1
Code Pull requests Activity

feat: add maxDepth to NodeSpec for bounded recursive nesting #93

Closed
jim-alexander wants to merge 1 commit from jim-alexander/prosemirror-model:feat/max-depth-node-spec into main
pull from: jim-alexander/prosemirror-model:feat/max-depth-node-spec
merge into: prosemirror:main
Conversation 3 Commits 1 Files changed 4 +355 -23
jim-alexander commented 2026-05-07 07:44:27 +02:00
First-time contributor
Copy link

Adds an optional maxDepth field to NodeSpec that limits how many times
a node type may appear in its own ancestor chain. A fast-path flag
Schema.hasMaxDepth is precomputed so schemas that don't use the feature
pay no runtime cost.

Enforcement is threaded through:

  • ContentMatch.matchType / matchFragment (via optional parentTypes param)
  • ContentMatch.findWrapping / computeWrapping (cache bypassed when
    parentTypes present; viaExceeds walks both document ancestor chain
    and BFS wrapping path without materialising a combined array)
  • Node.check() (accumulates ancestor chain recursively)
  • Node.replace() (runs check() on result when schema.hasMaxDepth is true)
  • Node.canReplace / canReplaceWith (enforce immediate parent context)

All new parameters are optional — existing call sites are unchanged and
all existing behaviour is preserved for schemas that don't declare maxDepth.

Adds an optional maxDepth field to NodeSpec that limits how many times a node type may appear in its own ancestor chain. A fast-path flag Schema.hasMaxDepth is precomputed so schemas that don't use the feature pay no runtime cost. Enforcement is threaded through: - ContentMatch.matchType / matchFragment (via optional parentTypes param) - ContentMatch.findWrapping / computeWrapping (cache bypassed when parentTypes present; viaExceeds walks both document ancestor chain and BFS wrapping path without materialising a combined array) - Node.check() (accumulates ancestor chain recursively) - Node.replace() (runs check() on result when schema.hasMaxDepth is true) - Node.canReplace / canReplaceWith (enforce immediate parent context) All new parameters are optional — existing call sites are unchanged and all existing behaviour is preserved for schemas that don't declare maxDepth.
Adds an optional maxDepth field to NodeSpec that limits how many times
a node type may appear in its own ancestor chain. A fast-path flag
Schema.hasMaxDepth is precomputed so schemas that don't use the feature
pay no runtime cost.

Enforcement is threaded through:
- ContentMatch.matchType / matchFragment (via optional parentTypes param)
- ContentMatch.findWrapping / computeWrapping (cache bypassed when
  parentTypes present; viaExceeds walks both document ancestor chain
  and BFS wrapping path without materialising a combined array)
- Node.check() (accumulates ancestor chain recursively)
- Node.replace() (runs check() on result when schema.hasMaxDepth is true)
- Node.canReplace / canReplaceWith (enforce immediate parent context)

All new parameters are optional — existing call sites are unchanged and
all existing behaviour is preserved for schemas that don't declare maxDepth.
marijn commented 2026-05-07 15:55:17 +02:00
Owner
Copy link

This would add another subtle limitation on where nodes can appear, and thus make it even more difficult to write correct generic document-manipulation code. Its motivation doesn't seem very compelling. I suggest you use a transaction appender to fix over-deep nesting, and constrain the user interface to make it harder for people to add such nodes at a level where they shouldn't appear, instead.

This would add another subtle limitation on where nodes can appear, and thus make it even more difficult to write correct generic document-manipulation code. Its motivation doesn't seem very compelling. I suggest you use a transaction appender to fix over-deep nesting, and constrain the user interface to make it harder for people to add such nodes at a level where they shouldn't appear, instead.
marijn closed this pull request 2026-05-07 15:55:17 +02:00
jim-alexander commented 2026-05-08 00:19:47 +02:00
Author
First-time contributor
Copy link

limitation on where nodes should appear

Yes that’s the whole point, it’s recursive currently.

transaction appender

I went into lots of detail about why I don’t want to rely on that pattern and why it makes sense for the layer to also have support for limiting, can you please read my accompanying doc here: https://discuss.prosemirror.net/t/feature-request-maxdepth-on-nodespec-upstream-prosemirror-pitch/9009/3

> limitation on where nodes should appear Yes that’s the whole point, it’s recursive currently. > transaction appender I went into lots of detail about why I don’t want to rely on that pattern and why it makes sense for the layer to also have support for limiting, can you please read my accompanying doc here: https://discuss.prosemirror.net/t/feature-request-maxdepth-on-nodespec-upstream-prosemirror-pitch/9009/3
jim-alexander reopened this pull request 2026-05-08 00:19:47 +02:00
marijn commented 2026-05-08 08:20:13 +02:00
Owner
Copy link

I went into lots of detail about why I don’t want to rely on that pattern

Yes, and I didn't find those motivations convincing. It's not going to be possible to express every possible constraint in the schema (see also table squareness), so transaction appenders and extra checks in the user interface are part of the toolkit you'll have to work with.

This feature is definitely not happening. It adds a non-local type of schema constraint, so that adding a node no longer just requires checking the parent and the node, but also the node's content. It makes content matches no longer a reliable source of truth about content fitting. And more generally it adds a kind of complexity that I very much am not interested in pulling into the library. You can fork, or you can work with the primitives provided by the library. I'm not merging this.

> I went into lots of detail about why I don’t want to rely on that pattern Yes, and I didn't find those motivations convincing. It's not going to be possible to express every possible constraint in the schema (see also table squareness), so transaction appenders and extra checks in the user interface are part of the toolkit you'll have to work with. This feature is definitely not happening. It adds a non-local type of schema constraint, so that adding a node no longer just requires checking the parent and the node, but also the node's content. It makes content matches no longer a reliable source of truth about content fitting. And more generally it adds a kind of complexity that I very much am not interested in pulling into the library. You can fork, or you can work with the primitives provided by the library. I'm not merging this.
marijn closed this pull request 2026-05-08 08:20:13 +02:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
prosemirror/prosemirror-model!93
No description provided.