Skip to content

fix: accept uppercase spellings of the standard option - #403

Open
rahul05ranjan wants to merge 1 commit into
avoidwork:masterfrom
rahul05ranjan:fix/standard-option-case
Open

rahul05ranjan wants to merge 1 commit into
avoidwork:masterfrom
rahul05ranjan:fix/standard-option-case

Conversation

@rahul05ranjan

Copy link
Copy Markdown

What

The JSDoc for the standard option advertises "SI, IEC, JEDEC", but the STANDARD_CONFIGS lookup is keyed on the lowercase "si" / "iec" / "jedec", so an uppercase spelling misses the lookup and silently falls through to the decimal default — the wrong unit system:

filesize(1048576, { standard: "iec" }); // "1 MiB"
filesize(1048576, { standard: "IEC" }); // "1.05 MB"  <- decimal fallback, no warning
filesize(1000, { standard: "SI" });     // "1.00 kB"  <- happens to look right, by accident

Fix

Case-normalize the value in getBaseConfiguration() before the cached lookup. Unknown values keep the existing fallback behavior, which is pinned by the should fall back to JEDEC default for unknown standard test, so this change only affects spellings that are case variants of the three real standards — exactly the ones the JSDoc shows. The same normalization covers partial({ standard: "IEC" }).

Verification

  • New tests in tests/unit/filesize.test.js (uppercase IEC/JEDEC/SI, the partial() path, and a guard that the unknown-value fallback is unchanged): 3 fail on main@d2bb4c1, all pass with this change.
  • Full suite with the fix: node --test tests/unit/ — 278 pass, 0 fail (273 pre-existing + 5 new).

The JSDoc for the `standard` option advertises "SI, IEC, JEDEC", but
STANDARD_CONFIGS is keyed on lowercase "si"/"iec"/"jedec", so any
uppercase spelling missed the lookup and silently fell through to the
decimal default:

  filesize(1048576, { standard: "iec" }) // "1 MiB"
  filesize(1048576, { standard: "IEC" }) // "1.05 MB" (wrong unit system)

Case-normalize the value in getBaseConfiguration() before the lookup.
Unknown values keep the existing fallback behavior (pinned by the
"should fall back to JEDEC default for unknown standard" test).

Co-authored-by: rahul05ranjan <11793455+rahul05ranjan@users.noreply.github.com>

This branch has not been deployed

No deployments
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