Editing store: drop dead previouslyEditedElement / tabDirection state (#334 follow-up)#353
Merged
Merged
Conversation
…te (#334 follow-up) After the redirect useLayoutEffect retirement, both `previouslyEditedElement` and `tabDirection` were write-only — `recordPreviousEdit` / `setTabDirection` had no readers. Audit confirmed nothing in src/ or test/ consumes either. Drops the two state fields from `EditingStateBundle`, the two action methods from `EditingStore`, the unused `TabDirection` import, and the corresponding calls from `ValueNodeWrapper.tabTo` (the viability-aware `getNextOrPreviousAtPath` already handles non-viable Tab targets up front). Public `useEditing` hook shape shrinks accordingly. Net: -22 lines on EditingProvider, -15 lines on ValueNodeWrapper (~37 removed, 0 added that count beyond moved comment text). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Bundle size impact
|
| Format | Base raw | PR raw | Δ raw | Base gzip | PR gzip | Δ gzip |
|---|---|---|---|---|---|---|
| esm | 56.03 KB | 55.76 KB | 🟢 -280 B (-0.49%) | 20.10 KB | 20.01 KB | 🟢 -89 B (-0.43%) |
| cjs | 57.56 KB | 57.29 KB | 🟢 -281 B (-0.48%) | 20.19 KB | 20.10 KB | 🟢 -91 B (-0.44%) |
Measured from build/index.{cjs,esm}.js. Gzip at level 9.
CarlosNZ
added a commit
that referenced
this pull request
Jun 12, 2026
…ct (#334) (#352) * Tab viability: skip non-viable targets in getNextOrPrevious (#334) Retires the redirect useLayoutEffect in ValueNodeWrapper. `getNextOrPrevious` gains an optional `isViable` predicate; `useCommon` composes it from `filterState.visiblePaths` (new `useRawFilterState` hook) and the existing `allowEditFilter`. Tab navigation skips filtered-out and non-editable nodes up front instead of opening and bouncing, eliminating the spurious startEdit/cancelEdit observer pairs on dead-end nodes. KeyDisplay's Tab path (key-edit → value-edit) inherits the same viability check, since both source roles share `getNextOrPreviousAtPath` via useCommon. Behaviour changes: - No `previouslyEditedElement` fallback when no viable next exists; the editor cancels cleanly instead. - Live search hiding the actively-edited node leaves the editing record in the store; the input unmounts via the existing `!isVisible` early-return. Clearing the search later resumes the edit. No off-screen-commit footgun because there's no input to commit through. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * PR #352 review: required isViable param + extract buildNodeData - `getNextOrPrevious`: drop the optional `?` on `isViable`. The only no- predicate callers were tests; production (`useCommon`) always supplies one. Tests now pass `acceptAll = () => true` for the pure structural walk. The "absence of the predicate is byte-equivalent" case is gone — trivially true under a required predicate. - `buildLeafNodeData` (inline in `keyboard.ts`) was a near-duplicate of `buildNodeData` in `JsonEditor.tsx`. Extracted the canonical builder to `src/utils/buildNodeData.ts` and reuse from both call sites. `rootName` defaults to `''` for callers that never target the root. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Editing store: remove dead previouslyEditedElement / tabDirection state (#334 follow-up) (#353) After the redirect useLayoutEffect retirement, both `previouslyEditedElement` and `tabDirection` were write-only — `recordPreviousEdit` / `setTabDirection` had no readers. Audit confirmed nothing in src/ or test/ consumes either. Drops the two state fields from `EditingStateBundle`, the two action methods from `EditingStore`, the unused `TabDirection` import, and the corresponding calls from `ValueNodeWrapper.tabTo` (the viability-aware `getNextOrPreviousAtPath` already handles non-viable Tab targets up front). Public `useEditing` hook shape shrinks accordingly. Net: -22 lines on EditingProvider, -15 lines on ValueNodeWrapper (~37 removed, 0 added that count beyond moved comment text). Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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
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.
Stacked on #352. Merge that one first.
Follow-up cleanup queued from #352's plan. After the redirect
useLayoutEffectretirement landed,previouslyEditedElementandtabDirectionbecame write-only state — the only writers (recordPreviousEdit,setTabDirection) had no remaining readers anywhere insrc/ortest/. Audit:Changes
EditingStateBundlelosespreviouslyEditedElement: CollectionKey[] | nullandtabDirection: TabDirection.EditingStorelosesrecordPreviousEditandsetTabDirection. Shape is now{ subscribe, getSnapshot, getServerSnapshot, open, cancel, submit, areChildrenBeingEdited }.ValueNodeWrapper.tabTodrops the two store calls. The viability-awaregetNextOrPreviousAtPath(from Tab viability: skip non-viable targets, retire redirect useLayoutEffect (#334) #352) already skips non-viable Tab targets up front, which is the role those store fields used to support.TabDirectionimport removed fromEditingProvider.useEditingpublic hook's return shape shrinks correspondingly (the two action methods are no longer exposed).Test plan
pnpm test— 456 tests across 18 suites passing (same as Tab viability: skip non-viable targets, retire redirect useLayoutEffect (#334) #352, no test references to the removed fields).pnpm lint && pnpm compile && pnpm build— clean.Diff stat
src/contexts/EditingProvider.tsxsrc/ValueNodeWrapper.tsx.changeset/*.md🤖 Generated with Claude Code