Synchronously create cell-scoped context key service#13797
Open
seeM wants to merge 1 commit into
Open
Conversation
|
E2E Tests 🚀 |
nstrayer
previously approved these changes
May 27, 2026
Contributor
nstrayer
left a comment
nstrayer
left a comment
There was a problem hiding this comment.
I like this. Part of me wants to complete the migration and put outputFocused, outputOverflows, outputImageTargeted and outputJsonTargeted into this class as well but I think that wouldnt actually improve the abstraction, just add more forwarding code.
Seems good to me!
Base automatically changed from
refactor/extract-context-key-declarations
to
main
May 27, 2026 19:50
seeM
force-pushed
the
refactor/cell-context-key-service
branch
from
3d6b004 to
523ba5e
Compare
Contributor
Author
|
Ah good point. It might be possible to push that logic (or at least some it) into the class too. I'll take a look |
Replaces the useCellContextKeys hook with a CellContextKeyManager that is created when the cell container is attached. This moves context key lifecycle management out of the React layer and into the cell model, keeping the scoped context key service on the cell instance for consumers to access directly.
seeM
force-pushed
the
refactor/cell-context-key-service
branch
from
523ba5e to
357e4f8
Compare
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
2 participants
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.
Toward #10466
The main change here is to create the cell-scoped context key in the callback ref i.e. synchronously during render, rather than async via React state & effects. While doing that, I took the opportunity to move cell context key management out of the React layer and into the model layer. (Let me know if you disagree.)
I believe callback refs synchronously block rendering, but the work done here is super cheap and shouldn't degrade performance. It will, however, let us scope the cell editor's context key service to the cell without having to wait a few render cycles for the cell context key service to become available (which is the fix for #10466). I'll make that fix in a separate PR.
Release Notes
New Features
Bug Fixes
Validation Steps
@:positron-notebooks @:web