Skip to content

Refactor callsign.js: regex correctness, boolean config, O(1) prefix lookup, tests & CI#27

Merged
phieri merged 4 commits intomainfrom
copilot/refactor-callsign-js-functionality
Mar 28, 2026
Merged

Refactor callsign.js: regex correctness, boolean config, O(1) prefix lookup, tests & CI#27
phieri merged 4 commits intomainfrom
copilot/refactor-callsign-js-functionality

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 28, 2026

Several correctness and performance bugs in src/callsign.js: accidental commas in regex character classes, a trailing-space hack for end-of-sentence matching, string-flag config comparisons, O(n) prefix scans, and unsafe innerHTML reads. No tests or CI existed to catch regressions.

src/callsign.js

  • Regex fixes: Remove stray commas from [A-Z,\d] in SEARCH_REGEX and PARTS_REGEX; replace trailing \s with \b (handles end-of-sentence call signs, removes ${text} hack)
  • RegExp construction: Use new RegExp(SEARCH_REGEX.source, 'g') instead of passing the RegExp object directly
  • Boolean config: getConfig() now returns real booleans; introduces DEFAULT_CONFIG constant to avoid duplication between the no-element fallback and the dataset-parsed path
  • DOM safety: Constructor reads (this.textContent || '').trim() instead of this.innerHTML
  • O(1) prefix lookup: Build Callsign._reversePrefixMap (prefix→ISO) once from PREFIX_TABLE; createFlagElement and isValidPrefix use it instead of Array.includes scans
  • getPhonetics: Rewritten as Array.from(letters).map(...).filter(Boolean).join(' ') — silently drops unknown characters rather than producing "undefined" in output
  • Resilience: Guard customElements.define with customElements.get check; expose Callsign on window for test access

Tests & CI

  • tests/callsign.test.js: New integration tests that import src/callsign.js into jsdom and exercise getPhonetics, isValidPrefix, and searchCallsigns DOM replacement against the real module
  • .github/workflows/ci.yml: New workflow running npm run lint + npm test on Node 20
  • package.json: Test script updated to NODE_OPTIONS=--experimental-vm-modules jest for ESM support
Original prompt

Summary

This PR refactors src/callsign.js to fix correctness bugs, improve performance, and add tests + CI. Changes are focused and backwards-compatible for typical browser usage.

Problem & goals

  • Fix RegExp construction that can throw / behave incorrectly when creating a new RegExp from an existing RegExp and flags.
  • Remove accidental commas from regex character classes and make SEARCH_REGEX more robust (use \b instead of trailing space).
  • Improve performance of prefix lookups by creating a reverse lookup map (prefix -> ISO) to avoid O(n) includes(...) scans.
  • Convert configuration dataset values from string flags ('true'/'false') to booleans to simplify checks.
  • Use safer DOM APIs (textContent instead of innerHTML for reading call-sign text).
  • Guard customElements.define to avoid re-definition errors when embedded multiple times.
  • Improve getPhonetics implementation for clarity and robustness.
  • Expose Callsign on window for test access in jsdom environment.
  • Add unit tests (jest + jsdom) to verify parsing, phonetics, and search behavior.
  • Add a GitHub Actions workflow to run lint and tests on push/PR.

Files to be changed / added

  1. Modify: src/callsign.js
  • Replace SEARCH_REGEX and PARTS_REGEX to remove commas from character classes and use word boundaries.
  • When creating a global regex in searchCallsigns, construct with new RegExp(SEARCH_REGEX.source, 'g') to avoid passing flags to RegExp constructed from another RegExp.
  • Use (this.textContent || '').trim() instead of this.innerHTML.trim() when reading the element content.
  • Convert config dataset values to booleans in getConfig(); return default boolean config when script element missing.
  • Use boolean checks for config.search when auto-initializing search behavior.
  • Build a reverse prefix->ISO Map (Callsign._reversePrefixMap) used by createFlagElement and isValidPrefix; change those methods to use the Map for O(1) lookup.
  • Rewrite getPhonetics using Array.from(...).map(...).filter(...).join(' ') for clarity.
  • Guard customElements.define: only define if not already defined.
  • Attach Callsign to window for testability: if (typeof window !== 'undefined') window.Callsign = Callsign;
  1. Add tests: tests/callsign.test.js
  • Tests (jest/jsdom) covering:
    • getPhonetics returns expected phonetic expansion for a sample input.
    • isValidPrefix returns true for a known prefix and false for an unknown value.
    • searchCallsigns replaces a text node containing a callsign with a call-sign element.
  1. Add CI workflow: .github/workflows/ci.yml
  • Node.js workflow that installs dependencies and runs npm run lint and npm test on pull_request and push to branches.
  1. package.json: no required dependency changes; keep devDependencies. No change required but tests will be added.

Implementation details (patch highlights)

  • Regex changes

    • SEARCH_REGEX: /([A-Z\d]{1,3}\d[A-Z]{1,3}(?:/\d)?)\b/
    • PARTS_REGEX: /([A-Z\d]{1,3})(\d)([A-Z]{1,3})(?:/(\d))?/
  • Regex creation inside searchCallsigns

    • const regex = new RegExp(SEARCH_REGEX.source, 'g');
    • while ((match = regex.exec(text)) !== null) { ... }
  • Config boolean coercion

    • flag: ds.flag !== 'false', monospace: ds.monospace !== 'false', phonetic: ds.phonetic !== 'false', search: ds.search === 'true'
  • Reverse prefix map

    • Callsign._reversePrefixMap = new Map(); populate once from PREFIX_TABLE.
    • createFlagElement(prefix) looks up iso = Callsign._reversePrefixMap.get(prefix)
    • isValidPrefix(prefix) checks Callsign._reversePrefixMap.has(prefix)
  • Accessibility and DOM

    • Use textContent to avoid HTML parsing surprises.
    • Guard customElements.define('call-sign', Callsign) with a customElements.get check.

Tests: example test file (tests/callsign.test.js)

  • Setup: require/import src/callsign.js in each test (module will define custom element and attach Callsign to window in jsdom).
  • Test cases for phonetics, isValidPrefix, and searchCallsigns DOM replacement.

CI workflow

  • Create .github/workflows/ci.yml to run Node 20 (or 18/20), install dependencies (npm ci), run npm run lint and npm test.

PR description

  • Title: "Refactor: regex fixes, config booleans, performance improvements, tests & CI"
  • Body: Short summary of above goals, list of changed files, testing/CI added, notes about backward compatibility: The library still defines the custom element and works when loaded by <script>. getConfig returns boolean flags now; dataset usage accepts 'false' to disable features (backwards compatible with prior 'false' checks). The export surface is unchanged for browser usage (we only attach Callsign to window for testability).

Please create a pull request in phieri/callsign.js implementing the listed changes, with a single commits (or a small logical set of commits) and include tests and a GitHub Actions workflow. Run lint and tests locally in the branch before opening the PR.

If anything is unclear or you want the changes split into multiple PRs (critical fixes first, then tests...

This pull request was created from Copilot chat.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI changed the title [WIP] Refactor callsign.js for correctness and performance improvements Refactor callsign.js: regex correctness, boolean config, O(1) prefix lookup, tests & CI Mar 28, 2026
Copilot AI requested a review from phieri March 28, 2026 21:38
@phieri phieri marked this pull request as ready for review March 28, 2026 21:48
@phieri
Copy link
Copy Markdown
Owner

phieri commented Mar 28, 2026

@copilot the CI pipeline failed. Check logs and fix.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 28, 2026

@copilot the CI pipeline failed. Check logs and fix.

Fixed in 69e2b6c. The CI workflow used npm ci and cache: 'npm', both of which require a package-lock.json — but that file is excluded by .gitignore (matching the existing test.yml). Replaced npm ci with npm install and removed the cache: 'npm' option.

@phieri phieri merged commit 615a823 into main Mar 28, 2026
4 checks passed
@phieri phieri deleted the copilot/refactor-callsign-js-functionality branch March 28, 2026 21:55
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.

2 participants