Repository navigation
Refactor/labelstore decomposition - #114
Merged
Merged
Conversation
Foundation for the labelStore slice decomposition (architecture-cleanup #3). Extracted to src/store/labelStore.internals.ts (~170 LOC): - LOCK_BYPASS_KEYS + isLockBypass - rewriteTemplateMarkers, applyObjectChanges, insertAt - detectLocale, detectInitialTheme, thirdPartyDefaults - DUPLICATE_OFFSET_DOTS, buildOffsetCopies, cloneChildrenFresh - updateCurrentObjects + PageState type Lifted persist + temporal partialize functions out of the inline create() call as named exports (persistPartialize, temporalPartialize). Enables direct tripwire testing without reflecting into the middleware. NEW src/store/labelStore.tripwire.test.ts asserts: - persist partialize key set (snapshot — slice extractions that drop a key from persist fail loudly here) - temporal undo-timeline key set (same) - public selectors stay exported + callable (currentObjects, canCallLabelary, selectLabelaryNoticeRequired, selectPreviewLocksEditor, selectBatchInputs, selectCanBatchExport) labelStore.ts shrunk from 1542 to 1393 LOC. 1417 tests pass (+3 tripwires).
First slice extracted from labelStore.ts. Pattern established for the 8 remaining slices. New: src/store/slices/printerProfileSlice.ts (65 LOC) - PrinterProfileSlice interface (printerProfile field + patch/reset actions) - createPrinterProfileSlice: StateCreator<LabelState, [], [], PrinterProfileSlice> - Cross-slice read via state arg uses the inherited LabelState (selectPreviewLocksEditor) labelStore.ts changes: - LabelState split into LabelStateBase + intersection with PrinterProfileSlice - create() initializer spreads ...createPrinterProfileSlice(set, get, store) - Inline printerProfile, patchPrinterProfile, resetPrinterProfile removed - Dropped now-unused imports: EMPTY_PRINTER_PROFILE, printerProfileSchema, PrinterProfile type, pruneUndefined (all moved to the slice) labelStore.ts: 1393 → 1339 LOC. 1417 tests pass (incl. the 3 tripwires asserting printerProfile stays in both persist and temporal partialize key sets).
New: src/store/slices/uiSlice.ts (121 LOC) - UiSlice interface: locale, theme, thirdParty, labelaryNoticeAcknowledged, canvasSettings, sidebarTab, printerSettingsTab, zebraPrintSource, editorFocusRequest + their 9 setters - Type homes moved with the state: CanvasSettings, ThemePreference, PrinterSettingsTab, SidebarTab now live in uiSlice.ts - createUiSlice uses detectLocale/detectInitialTheme/thirdPartyDefaults from labelStore.internals for initial values labelStore.ts changes: - LabelState = LabelStateBase & PrinterProfileSlice & UiSlice - LabelStateBase drops 5 UI fields (locale, theme, canvasSettings, thirdParty, labelaryNoticeAcknowledged) plus sidebarTab et al. - create() initializer drops the 9 UI inline definitions and the 5 initial values; spreads createUiSlice(set, get, store) - Dropped now-unused imports: LocaleCode, detectLocale, detectInitialTheme, thirdPartyDefaults PrinterSettingsModal.tsx: import PrinterSettingsTab from the slice directly (no re-export shim per 'kein legacy' guidance). labelStore.ts: 1339 → 1252 LOC. 1417 tests pass (tripwires still pass — canvasSettings, labelaryNoticeAcknowledged, locale, theme all stay in persist key set).
New: src/store/slices/selectionSlice.ts (62 LOC) - SelectionSlice interface: selectedIds + selectObject / toggleSelectObject / selectObjects / removeSelectedObjects - removeSelectedObjects is a cross-slice writer (touches pages via updateCurrentObjects + selectedIds) — owned here because the Delete-key trigger lives with the selection it acts on labelStore.ts changes: - LabelState += SelectionSlice - selectedIds field + 4 actions removed from LabelStateBase - create() spread adds createSelectionSlice; initial selectedIds: [] dropped (slice owns the default) labelStore.ts: 1252 → 1196 LOC. 1417 tests pass.
…wire+ Architecture review of Stages 0-3 flagged: - P0 type duplication (CanvasSettings/ThemePreference/PrinterSettingsTab declared in both labelStore.ts and slices/uiSlice.ts after Stage 2) - P1 tripwire missing getCurrentObjects/useCurrentObjects/useHistory - P2 slice-to-labelStore runtime import is a cycle-class hazard for the remaining 5 slices Fixes: - Delete the orphan CanvasSettings/ThemePreference/PrinterSettingsTab declarations from labelStore.ts (canonical lives in uiSlice.ts) - NEW src/store/labelStore.selectors.ts (49 LOC): currentObjects, canCallLabelary, selectLabelaryNoticeRequired, selectPreviewLocksEditor, selectBatchInputs, selectCanBatchExport - labelStore.ts re-exports them for external consumers (no API churn) - Slices import directly from labelStore.selectors — eliminates the runtime-cycle hazard before Stages 4-8 amplify it - Tripwire EXPECTED_PUBLIC_SELECTORS adds getCurrentObjects, useCurrentObjects, useHistory; header essay trimmed to one line - Dropped now-unused imports: Unit, ViewRotation, isDefaultLabelaryHost, PageState 1417 tests pass, lint and build green.
New: src/store/slices/previewSlice.ts (110 LOC) - PreviewMode type (discriminated union: idle | loading | active | error) - previewCache closure + __resetPreviewCacheForTests (module-scoped to preserve the non-serialisable blob URL invariant; persisting it would resurrect a stale identifier on reload) - PreviewSlice interface: previewMode + enterPreviewMode + exitPreviewMode - enterPreviewMode is async with the existing isStale-guard pattern (status check + label/objects reference equality) labelStore.ts changes: - LabelState += PreviewSlice - LabelStateBase drops previewMode field + 2 actions - create() spread adds createPreviewSlice; inline initial previewMode dropped - re-exports __resetPreviewCacheForTests for the existing label-store test that uses it (consumer API stability) - Dropped now-unused imports: fetchPreview, labelaryErrorMessage, buildActiveCsvRow, buildPreviewZpl (all moved into the slice) labelStore.ts: 1196 → 1023 LOC. 1417 tests pass (incl. the existing preview-cache cache-hit / cache-miss tests in labelStore.test.ts — they import __resetPreviewCacheForTests via the labelStore re-export, no test touched).
New: src/store/slices/csvSlice.ts (126 LOC) - CsvDataset type moved here (was in labelStore.ts) - CsvSlice interface: csvDataset, csvMapping, csvMappingModalOpen + loadCsv, clearCsv, setActiveRow, setCsvMapping, applyMappingDraft, openCsvMappingModal, closeCsvMappingModal - applyMappingDraft is a cross-slice writer (touches variables) — owned here because the mapping-modal Apply path is the trigger labelStore.ts changes: - LabelState += CsvSlice - LabelStateBase drops 3 fields + 7 actions + the CsvDataset interface - create() spread adds createCsvSlice; 3 initial values dropped - forgetImport import re-added (loadDesign still uses it; will move with labelConfigSlice in Stage 7) labelStore.selectors.ts: CsvDataset type-import switched from labelStore to slices/csvSlice (selectors keep state-wide visibility but type comes from the slice that owns it). labelStore.ts: 1023 → 899 LOC (under 1k for the first time).
New: src/store/slices/variablesSlice.ts (133 LOC) - VariablesSlice interface: variables + add/update/remove/setVariables - Cross-slice writes: removeVariable strips bindings on pages and drops csvMapping entries; updateVariable rewrites template markers across pages on rename NEW types/Variable.ts: validateVariablesUnique(variables) helper - Promotes the inline bulk-replace validation pattern (4× duplicated loop checking trimmed name + fnNumber uniqueness + range) into a named predicate - setVariables (variablesSlice) and applyMappingDraft (csvSlice) both call it instead of inlining the loop — debt mitnehmen per the plan labelStore.ts: - LabelState += VariablesSlice - LabelStateBase drops variables field + 4 actions - create() spread adds createVariablesSlice - Drops now-unused imports: nextFreeFnNumber, FN_NUMBER_MIN/MAX, stripVariableIdFromObjects, rewriteTemplateMarkers labelStore.ts: 899 → 751 LOC. 1417 tests pass.
Round-1 findings addressed:
- P1 (Architecture + Code+Smells): move __resetPreviewCacheForTests
re-export out of the import block in labelStore.ts onto its own line
below the imports.
- P2 (Code+Smells): updateVariable's param-mutation (changes = {...})
replaced with a local 'patched' binding.
- P2 (Code+Smells): LabelState intersection spread across multiple lines.
- Note: _drop / void _drop kept in removeVariable — the project's eslint
config doesn't ignore _-prefixed unused destructure bindings.
Comment trims (4 items):
- previewSlice.ts: PreviewMode JSDoc trimmed (essay -> 2-line WHY)
- previewSlice.ts: previewCache header trimmed (drop 'closure owns URL'
paragraph — set() body is self-documenting)
- previewSlice.ts: PreviewSlice interface field docs trimmed (drop
PreviewMode-status restatement; the type's docstring covers it)
- previewSlice.ts: exitPreviewMode inline comment removed (already on
interface docstring)
- csvSlice.ts: csvDataset 5-line persist-rationale -> 1-line
1417 tests pass.
New: src/store/slices/labelConfigSlice.ts (66 LOC) - LabelConfigSlice interface: label + setLabelConfig + loadDesign + appendPages - loadDesign is the canonical 'atomic file open' — single set() resets label / pages / currentPageIndex / selectedIds / variables / csvMapping / csvDataset (5 slices' fields). Cross-slice writes are the operation here, not a side effect. - forgetImport (CSV module cache) ships with loadDesign now. labelStore.ts: - LabelState += LabelConfigSlice - LabelStateBase drops label field + 3 actions - create() spread adds createLabelConfigSlice; inline label default dropped (slice owns it) - Dropped now-unused imports: LabelConfig type, CsvMapping type, forgetImport (moved into the slice) labelStore.ts: 751 → 705 LOC. 1417 tests pass.
… pure composition New: src/store/slices/objectSlice.ts (475 LOC) - ObjectSlice interface: pages, currentPageIndex, clipboard, pasteCount + 21 actions covering object CRUD, group/ungroup, reparent, z-order, copy/paste, and page management - All 21 actions operate on pages[currentPageIndex].objects via updateCurrentObjects (or manage the page list directly) labelStore.ts: - LabelState is now a pure intersection of 8 slices (no LabelStateBase remnant — every field lives in a slice) - create() body is just 8 spread calls; no inline state, no inline actions - Dropped now-unused imports: Group helpers (mapObjectById, detachObjectById, findObjectById, findAncestors, isSelfOrDescendant, isGroup, GroupObject, Page), internals helpers (applyObjectChanges, insertAt, DUPLICATE_OFFSET_DOTS, buildOffsetCopies, cloneChildrenFresh, updateCurrentObjects), getEntry from registry labelStore.ts: 705 → 218 LOC (target: pure composition, hit). Decomposition complete: printerProfile 66 LOC (1 field, 2 actions) ui 121 LOC (9 fields, 9 actions) selection 60 LOC (1 field, 4 actions) preview 95 LOC (1 field, 2 actions + module cache) csv 110 LOC (3 fields, 7 actions) variables 134 LOC (1 field, 4 actions) labelConfig 66 LOC (1 field, 3 actions) object 475 LOC (4 fields, 21 actions) ──────────────────────── total slices 1127 LOC labelStore.ts 218 LOC (composition + migrations + history hook) 1417 tests pass.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the monolithic labelStore.ts by modularizing its state and actions into smaller, domain-specific slices under src/store/slices/, while extracting internal helpers and selectors into dedicated files. It also introduces a tripwire test suite to guarantee API stability. The review feedback highlights potential runtime crash risks in non-browser or non-Vite environments (such as SSR or test runners) and suggests safely guarding references to global objects like navigator, window.matchMedia, and import.meta.env.
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.
No description provided.