Repository navigation
Cleanup/post printer profile audit - #111
Merged
Merged
Conversation
Five themes folded into one commit: 1. designFile: require explicit \`schemaVersion\` (= 1) on the exported design-file JSON, drop the two legacy fallback shapes (pre-version pages-only + pre-multipage objects-only). The project is young; carrying those branches forward would mean a v1→v2 migration later having to dispatch over three shapes instead of one. Files saved by older builds now fail loud (invalid_schema) instead of silently upgrading. 2. Visual-regression re-framing: the text visual suite uploads PrintLabZPL-Bold.ttf to Labelary and registers the same TTF locally with @napi-rs/canvas, so both sides render with identical glyph data. The test measures Skia-vs-Labelary anti-aliasing drift, NOT firmware fidelity. Renamed and re-documented to reflect what it actually checks. 3. Comment trim across the printer-profile epic files (zplSetupScript, PrinterProfile, PrinterSettingsModal, pruneUndefined, useCopyToClipboard) — drop multi-paragraph module headers and per-field docstrings that restate the spec; keep firmware-quirk anchors (^TA padding, ^KN trim, ^SL three- shape), schema cross-field rule, live-clock mode rationale. ~120 lines of retroactive review-prose removed. 4. Dead-code: two locale keys orphaned by the modal refactor (\`output.setupScriptHeading\`, \`output.hiddenSecondaryHint\`) removed across all 32 locales; \`src/registry/_template.ts\` scaffold-only file removed (never imported, no value as documentation — contributors can copy any concrete registry entry instead). 5. API-surface trim: 8 internal-only exports demoted from \`export\` to local scope so the public module surface tracks what's actually consumed elsewhere — LabelaryError, partialLoss, AUTO_SNAP_TOLERANCE_DEG, GRAPHIC_EXT, LIGHT_COLORS, ROOT_CONTAINER, toCode128BRaw, commitRotatedSizeTransform.
…ser tripwires
Two themes:
1. Type-safety: 9 of the 30 lint warnings flagged unjustified \`!\`
non-null assertions. Replaced with proper narrowing.
- TemplateContentInput.tsx (8 sites) — extract a
\`caretOffsetIn(editor, sel, which)\` helper that does the
"Selection's anchor/focus Node | null after editor.contains()"
narrowing once. Original code reached for \`!\` because
TypeScript can't follow \`contains()\` as a narrowing.
- contentEditableCaret.ts (2 sites) — \`el.parentNode!\` replaced
with explicit \`if (!parent) return\` guards.
- zplParser.ts:1625 — dropped one \`as unknown as Record<string,
unknown>\` indirection. After \`lastObj.type === "text"\` narrowed,
\`lastObj.props\` is already \`TextProps\`; the indirection was
masking the discriminated union.
Also demoted zplCommandSupport.ts internal exports
(ZplCommandStatus, ZplCommandInfo, ZPL_COMMANDS) to non-exported
and deleted the unused getCommandStatus() helper.
2. Parser pre-refactor tripwires for the upcoming pipeline split:
- zplParser.test.ts: emits-one-finding-per-kind test now asserts
source-order of finding.command. A split that collects
unknowns / browser-limits in a separate post-pass would
reorder these and break the assertion.
- zplRoundtrip.test.ts: shipping-label round-trip now asserts no
labelConfig key dropped between first parse and re-parse. A
labelConfig fold running across blocks instead of per-block
would drop keys here. Subset-check (not equality) because the
generator legitimately gains widthMm/heightMm defaults.
1408 tests pass; 21 lint warnings remaining (down from 30).
…b/labelGeometry Architecture epic #2 — Parser → Canvas decoupling. The ZPL parser (a pure Domain module) imported \`zplAnchorToModel\` and \`computeTextRenderMetrics\` from \`components/Canvas\`. Same for \`registry/zplHelpers.ts\` and \`registry/text.tsx\`'s ZPL-emit path. This made Domain code transitively depend on a folder named for UI. The three files are in fact pure logic: - textPositionTransforms.ts — anchor / FO / FT geometry, no DOM - measureTextDots.ts — DOM-canvas measureText with non-DOM fallback - textRenderMetrics.ts — combines the above + font helpers from lib/ None import Konva, React, or any Canvas-folder component. The "Canvas/" location was historical (Konva renderer was their first consumer); they belong in lib/ where every consumer can reach them without crossing the UI boundary. Move: - src/components/Canvas/textPositionTransforms{,.test}.ts - src/components/Canvas/textRenderMetrics.ts - src/components/Canvas/measureTextDots.ts → src/lib/labelGeometry/ Importers updated: - src/lib/zplParser.ts - src/registry/text.tsx - src/registry/zplHelpers.ts - src/components/Canvas/KonvaObject.tsx (still uses them, now via lib path) Single source of truth preserved (same module, just relocated). Tests + build green; no behavior change.
Architecture epic #4 — split the 2466-LOC parser monolith into focused modules under \`src/lib/zplParser/\`. Backward-compatible re-exports keep \`import from "../lib/zplParser"\` working unchanged across all 12 consumer test files (~362 tests) without import-path updates. Stage 1-2: pure functions out. - types.ts (67 LOC) — ImportFinding{Kind}, ImportReport, ParsedZPL, Handler, DecodedGraphic, UploadedGraphic - helpers.ts (117 LOC) — tokenize, int, inRange, strParam, makeObj, variableNameFromComment, ciToEncoding, getDecoder, decodeFH - decoders/crc.ts (79 LOC) — crc16Xmodem, base64ToBytes, parseGfWrapper - decoders/gfa.ts (139 LOC) — tryInflateZlib, gfaHexToBytes, decompressGFA, gfPayloadToBytes - decoders/graphic.ts (65 LOC) — decodeGraphicToImage (the only decoder module that touches the DOM) Stage 3 (partial): handler-family pattern proven across 3 cohesive groups, each takes only the slice it actually mutates. Orchestrator merges via \`Object.assign(handlers, createFamilyHandlers(...))\`. - handlers/setupScript.ts (112 LOC) — JZ/JT/~TA/ST/KD/KL/SE/SZ/KN/SL, all printerProfile mutators - handlers/labelConfig.ts (115 LOC) — PW/LL/PQ/MM/LS/PR/MD/MT/MN/ ML/MF/XB/PO/PM/~SD, labelConfig + dpmm - handlers/unsupported.ts (49 LOC) — 13 noops + 5 browser-limit factories (FL/HT/LF/IM/~DG), append-only sinks Dependency graph stays strictly acyclic: types ─┐ helpers┤ crc ──→ gfa ──→ graphic ──→ zplParser.ts ──→ handlers/* ↑ └─ orchestrator merges all zplParser.ts shrinks 2466 → 1757 LOC (−709, −29%). 743 LOC moved to 8 focused modules. No behavior change; 1408 tests pass. Remaining work in epic #4 (out of scope here, eigene Session): - ParserContext extraction (~520 LOC of closure-state move; the fundamental "context.ts" file the planner outlined as Stage 3) - Per-family extraction of barcodes (~210), graphics (~280), fields (~380) — each needs ParserContext to land first - flushField (~445 LOC, gravitational center of the closure-state)
![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 ZPL parser by modularizing zplParser.ts into several smaller files under src/lib/zplParser/ (including helpers, decoders, handlers, and types). It also introduces a schemaVersion field to the design file schema to enforce versioning and reject invalid or unknown schemas. Additionally, comments and unused exports are cleaned up across multiple files. The review feedback highlights a potential runtime TypeError in TemplateContentInput.tsx where the result of findCaretPosition is accessed without a safety check to ensure it is defined.
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.