Skip to content

fix(time-signature): return NONE instead of throwing on non-matching input - #501

Open
youdie006 wants to merge 1 commit into
tonaljs:mainfrom
youdie006:fix/490-parse-signature-guard
Open

fix(time-signature): return NONE instead of throwing on non-matching input#501
youdie006 wants to merge 1 commit into
tonaljs:mainfrom
youdie006:fix/490-parse-signature-guard

Conversation

@youdie006

Copy link
Copy Markdown

Fixes #490

Problem

TimeSignature.get / parse throw TypeError: Cannot read properties of undefined (reading 'split') on any string that doesn't match the <n>/<n> regex -- most notably "C" (common time), but also "", "x", etc.

Root cause

In parse, const [_, up, low] = REGEX.exec(literal) || [] yields an undefined up when the regex doesn't match. That recurses into the array branch where up.split("+") (index.ts:72) throws.

Fix

Add a defensive guard: when up is not a string (regex miss), return the zeroed parse [0, 0], which the existing build() already maps to the NONE / empty signature -- the same path invalid inputs like "0/0" already take. No behavior change for valid input.

Tests

Added a regression test in packages/time-signature/test.ts asserting that "C", "", "x", "4", "/", "abc" do not throw and return the empty signature, and that "4/4" still parses correctly.

  • Reverting the fix -> the new test reproduces the exact Cannot read properties of undefined (reading 'split') error (red).
  • With the fix -> package tests 8/8 pass; full suite 283/283 pass; lint and tsdown build clean (green).

This contribution was prepared with AI assistance.

…input

TimeSignature.get / parse threw 'Cannot read properties of undefined (reading
split)' on any string that does not match the <n>/<n> regex - most notably 'C'
(common time), but also '', 'x', etc. In parse, const [_, up, low] =
REGEX.exec(literal) || [] yields an undefined up on a regex miss, which recurses
into the array branch where up.split('+') throws.

Add a defensive guard: when up is not a string, return the zeroed parse [0, 0],
which build() already maps to the NONE / empty signature - the same path invalid
inputs like '0/0' already take. No behavior change for valid input.

Fixes tonaljs#490
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.

time-signature package fails on common time

1 participant