fix(@stdlib/string/base/atob): guard against missing global atob#13286
Draft
Planeshifter wants to merge 2 commits into
Draft
fix(@stdlib/string/base/atob): guard against missing global atob#13286Planeshifter wants to merge 2 commits into
atob#13286Planeshifter wants to merge 2 commits into
Conversation
The job `linux_test (Node.js v12)` and `linux_test (Node.js v14)` on workflow `linux_test` failed on develop with `ReferenceError: atob is not defined` at `lib/global.js:23`. Root cause: the module bare- referenced the global `atob` at load time, which throws on any Node.js version lacking that global (added in Node.js 16). Since `lib/index.js` requires `./main.js` (and transitively `./global.js`) unconditionally before its `hasAtobSupport()` feature-detection runs, this crashed on `require()` in production on Node.js <16, not just in tests. This commit guards the reference with `typeof atob === 'function'`, which does not throw on an undeclared identifier, matching the existing pattern in `@stdlib/assert/has-atob-support/lib/atob.js`. The module now safely resolves to `null` on unsupported platforms, letting the try/catch in `main.js` and the `hasAtobSupport()` check in `index.js` work as intended. Ref: https://github.com/stdlib-js/stdlib/actions/runs/28736797063
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
…arded `atob` reference CI check `Lint Changed Files` failed on this PR with two `n/no-unsupported-features/node-builtins` errors on the `typeof atob` guard added in the previous commit: the repo's configured Node.js version floor (>=0.12.18) predates the `atob` global (added in Node.js 16), and eslint-plugin-n flags the reference regardless of the `typeof` guard protecting it at runtime. This adds the same `eslint-disable-line` used for identical version-gated feature-detection elsewhere in the codebase, e.g. `@stdlib/os/homedir/lib/index.js` and `@stdlib/process/geteuid/lib/native.js`. Ref: https://github.com/stdlib-js/stdlib/actions/runs/28743835208
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.
Description
This pull request:
ReferenceError: atob is not definedin@stdlib/string/base/atobon Node.js versions lacking the globalatob(<16).lib/global.jsbare-referencedatob, which throws at module-load time. Sincelib/index.jsrequires./main.js(and transitively./global.js) unconditionally before running itshasAtobSupport()feature detection, this crashedrequire('@stdlib/string/base/atob')outright on Node.js <16, not just in tests.typeof atob === 'function', which does not throw on an undeclared identifier, matching the existing pattern in@stdlib/assert/has-atob-support/lib/atob.js. On unsupported platforms, the module now resolves tonull, and the existing try/catch inmain.jsplus thehasAtobSupport()check inindex.jshandle the rest as originally intended.Related Issues
None.
Questions
No.
Other
Failing run: https://github.com/stdlib-js/stdlib/actions/runs/28736797063 (
linux_test, jobs "Node.js v12" and "Node.js v14")Symptom:
ReferenceError: atob is not definedatlib/node_modules/@stdlib/string/base/atob/lib/global.js:23:18, thrown fromtest/test.main.jsonrequire, before tape'sskipoption ever takes effect.Root cause: bare (unguarded) reference to the global
atobinglobal.js, executed unconditionally at module load viaindex.js→main.js→global.js, regardless of the package's ownhasAtobSupport()feature detection.Validation: confirmed via manual
node -eruns that (1) requiringlib/main.jswithglobalThis.atobdeleted no longer throws and now returnsnullon invocation, (2) requiringlib/index.jsin the same simulated environment correctly falls back to the polyfill and returns the correct decoded string, (3) normal behavior on a Node.js version with nativeatobis unchanged. A fullnode_modulesinstall was not available in this environment to run the package'stapesuite directly. Reviewed independently by three agents (correctness, regression scope, style/conventions) — all approved, no blocking findings.Checklist
AI Assistance
Yes
No
Code generation (e.g., when writing an implementation or fixing a bug)
Test/benchmark generation
Documentation (including examples)
Research and understanding
Disclosure
This PR was authored by Claude Code as part of an automated CI-failure investigation routine: it identified the failing nightly job, root-caused the
ReferenceError, applied the fix, and validated it against three independent review passes before opening this draft.@stdlib-js/reviewers
Generated by Claude Code