Repository navigation
chore: robustness & DX hardening (#13) - #24
Merged
Merged
Conversation
Addresses the grab-bag of smaller robustness / DX items: - Invalid symbol IDs: `getSymbolId` now validates against an XML NCName pattern, rejecting XML-invalid names like `foo!` that would break `<use href="#foo!">` (previously only leading digits / whitespace were caught). - Nested folders: the input directory is walked recursively, so icons in sub-folders are included. Duplicate ids across folders (ids come from filenames) are rejected with a clear error. - Missing/empty viewBox: derived from width/height when possible, otherwise a warning is emitted and the attribute is omitted rather than written as an invalid `viewBox=""`. - Duplicate work: the input directory is read once and each file parsed once, then reused for both the sprite and the id list (was read/parsed twice). - Watch mode: documented a `chokidar-cli` / `concurrently` recipe in the README. - Node version: added an `engines` field to the published package and documented the minimum (Node >=18.20 or >=20.10 — the compiled CLI uses JSON import attributes, verified empirically that 20.0–20.9 fail). Updates and adds tests (`create.spec.ts`, new `directory.spec.ts`). Closes #13 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review follow-up: with the default `-o` (output === input) or an output dir nested under the input dir, the recursive walk would pick up a previously-written `symbolstore.svg` on the next run and emit a bogus `symbolstore` symbol (or throw a duplicate-id error). Exclude generated sprites (`symbolstore*.svg`) living in the output dir from the input walk. Adds a test covering output === input. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Resolve the bin/symbol-store.ts conflict by combining this branch's recursive icon walk + duplicate-id guard + single-parse with main's content hash (#8) and accessible UseSvg template (#10): - keep collectSvgFiles (recursive, already sorted) + the output-dir sprite guard + the single parse reused for both the sprite and SYMBOL_IDS (via getSvgSymbol), - keep useHashSuffix and the content-hash filename, - keep the accessible generated UseSvg (role=img / aria-hidden, title prop). README auto-merged (node-version/watch/recursive notes alongside the --hash and Accessibility sections). Co-Authored-By: Claude Opus 4.8 <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.
Closes #13
Grab-bag of smaller robustness / DX items, each independent:
getSymbolIdnow validates an XML NCName (/^[A-Za-z_][A-Za-z0-9._-]*$/), rejecting names likefoo!that break<use href="#foo!">. Test atcreate.spec.tsflipped from assertingfoo!valid → asserting it throws.viewBoxwidth/heightwhen possible; otherwise a warning is emitted and the attribute is omitted rather than written as invalidviewBox="".chokidar-cli/concurrentlyrecipe in the README (CLI stays a one-shot build step; no new dependency).enginesto the published package and documented the floor.Node floor (verified empirically)
The compiled CLI uses
import … with { type: "json" }, so I ran the builtdistacross versions:SyntaxError: Unexpected token 'with'→
engines.node:">=18.20.0 <19 || >=20.10.0".Verification
yarn test(package): 11 passing — incl. updatedcreate.spec.ts(NCName + omitted viewBox) and newdirectory.spec.ts(nested folders + duplicate-id error).tsc --noEmitgreen.🤖 Generated with Claude Code