Skip to content

fix(site-kit): flag data on a ./flags subpath + review follow-ups - #32

Merged
jlc488 merged 1 commit into
mainfrom
fix/site-kit-flags-subpath
Sep 2, 2026
Merged

fix(site-kit): flag data on a ./flags subpath + review follow-ups#32
jlc488 merged 1 commit into
mainfrom
fix/site-kit-flags-subpath

Conversation

@jlc488

@jlc488 jlc488 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fix-wave for the site-kit flag locale menu (#30, #31 → main 90786b1, unpublished 0.4.0), addressing final-review findings.

Fixes

  1. Flag data off the root barrel. FLAG_COUNTRY/LOCALE_FLAGS/flagFor no longer re-export from src/core/index.mjs/.d.mts; added "./flags" to package.json exports (types/importsrc/core/flags.d.mts/.mjs). src/solid/locale-menu.tsx already imported ../core/flags.mjs directly, so it's untouched. Updated tests/site-kit-contracts.test.mjs (barrel must NOT match flagFor; asserts the ./flags export exists with the right types/import) and both READMEs to describe the flag data as living on @devslab/site-kit/flags, with the size rationale. tests/site-kit-core.test.mjs already imported flags.mjs directly — no change needed there.

  2. Generator guard. scripts/build-flags.mjs now rejects a body containing a single-quoted id=' or any id="…" whose value doesn't match /^[\w-]+$/, throwing with the offending country. Generated header now also credits Copyright (c) 2013 Panayiotis Lipiridis. Regenerated flags.mjs/flags.d.mts; --check passes.

  3. SSR artwork assertion. ssr.test.tsx's flag-variant test now asserts viewBox="0 0 640 480" and a stable d="…" path fragment pulled from LOCALE_FLAGS.ko.body actually appear in the rendered HTML. Removed the vacuous not.toContain("<script") check.

  4. Trigger accessible name. The flag trigger's aria-label is now `${props.messages.localeLabel}: ${current().nativeName}` (e.g. "Language: English"), title unchanged. Updated locale-menu.test.tsx and the tests/browser/site-kit.spec.ts Arabic fixture ("اللغة: العربية") + assertion.

  5. Release verifier. scripts/verify-site-kit-release.mjs now checks src/core/flags.mjs, src/core/flags.d.mts, and flags/LICENSE-flag-icons.txt ship in the packed tarball, asserts flagFor is a function when importing flags.mjs directly, and spawns a probe script from the installed tarball's node_modules to prove @devslab/site-kit/flags resolves via the package's exports map.

Versioning note

0.4.0 (the flag locale menu itself) hasn't been published yet. Added .changeset/flags-subpath.md (@devslab/site-kit: patch) — since site-kit is in the fixed release group, this bump means the release ships as 0.4.1, with the flag menu and this follow-up going out together. Called out explicitly in the changeset text.

Verification (all green)

  • pnpm run build
  • node --test tests/site-kit-core.test.mjs tests/site-kit-contracts.test.mjs — 16 pass
  • pnpm --filter @devslab/site-kit run test — 17 pass (15 + 2 SSR)
  • pnpm --filter @devslab/site-kit run check
  • pnpm run verify:site-kit:browser — 6 pass
  • pnpm run verify:site-kit:release
  • pnpm run verify

…or, SSR test, a11y name, release verifier

- Move FLAG_COUNTRY/LOCALE_FLAGS/flagFor off the root barrel (src/core/index.mjs)
  onto a dedicated @devslab/site-kit/flags export, so consumers that never render
  a flag menu stop bundling ~110 KB of vendored SVG.
- Harden the flag generator: reject single-quoted id attributes and any id value
  that doesn't match /^[\w-]+$/, and credit the flag-icons copyright holder in
  the generated header.
- Assert the SSR flag-variant test actually renders artwork (viewBox + a stable
  path fragment from the Korean flag), not just markup shape.
- Give the flag trigger a self-describing aria-label ("Language: English")
  instead of the bare native name.
- Extend verify-site-kit-release.mjs to check the new files ship and that the
  ./flags subpath resolves from a fresh install.
@jlc488
jlc488 merged commit 12edb60 into main Sep 2, 2026
2 checks passed
@jlc488
jlc488 deleted the fix/site-kit-flags-subpath branch September 2, 2026 14:06
@jlc488 jlc488 mentioned this pull request Sep 2, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant