Skip to content

fix(@stdlib/string/base/atob): guard global atob reference for Node <16#13453

Closed
Planeshifter wants to merge 2 commits into
developfrom
philipp/ci-fix-atob-reference-error-2026-07-13
Closed

fix(@stdlib/string/base/atob): guard global atob reference for Node <16#13453
Planeshifter wants to merge 2 commits into
developfrom
philipp/ci-fix-atob-reference-error-2026-07-13

Conversation

@Planeshifter

Copy link
Copy Markdown
Member

Description

What is the purpose of this pull request?

This pull request:

  • Fixes linux_test (Node.js v12/v14 legs), which has failed on develop every scheduled run for the past month with ReferenceError: atob is not defined in lib/node_modules/@stdlib/string/base/atob/lib/global.js. Root cause: the file exported a bare reference to the global atob identifier. Under 'use strict', that throws a ReferenceError at evaluation time on any JS engine lacking atob as a global — Node.js only added atob/btoa as globals in v16.0.0. Because lib/index.js unconditionally requires ./main.js (which requires ./global.js) before ever checking hasAtobSupport(), simply require('@stdlib/string/base/atob') crashed on Node <16, defeating the package's own polyfill fallback.
  • Guards the reference with typeof atob === 'function', mirroring the identical pattern already used by this package's own dependency, @stdlib/assert/has-atob-support/lib/atob.js. typeof on an undeclared identifier evaluates to 'undefined' rather than throwing, so the module now loads safely on Node <16 and correctly falls back to the polyfill. Behavior on Node >=16 is unchanged.

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

Failing run: https://github.com/stdlib-js/stdlib/actions/runs/29241732795 (Node.js v12/v14 legs; the v16 leg in the same run fails separately, tracked in a different PR).

Symptom: ReferenceError: atob is not defined at lib/node_modules/@stdlib/string/base/atob/lib/global.js:23.

Validation: manually verified in a Node.js v22 sandbox (package installation was not available in this environment, so make lint-pkg/the package test command could not be run directly):

  • require('.../lib/global.js') with atob present returns the native function unchanged and decodes correctly.
  • delete globalThis.atob; require('.../lib/global.js') no longer throws (previously threw immediately).
  • delete globalThis.atob; require('@stdlib/string/base/atob') now correctly falls back to the polyfill and decodes 'SGVsbG8sIHdvcmxk' to 'Hello, world'.

Reviewed in three passes: correctness (confirmed the fix eliminates the require-time throw on Node <16 while leaving Node >=16 behavior identical; confirmed no other unguarded global reference remains in the package's require chain), regression scope (confirmed this is the only file changed; confirmed no other package imports lib/global.js directly; confirmed main.js's exported function is only ever invoked when hasAtobSupport() is true, so behavior for real callers is unchanged), and style/conventions (confirmed the pattern matches has-atob-support/lib/atob.js; confirmed commit type fix(@stdlib/string/base/atob) is correct per docs/style-guides/git/README.md).

Reviewer notes

Non-blocking: the fallback value uses void 0 where the mirrored has-atob-support/lib/atob.js uses null. Both are falsy and produce identical behavior on the unsupported path (the value is never read — main.js's exported function is never invoked when hasAtobSupport() is false), so this was left as-is rather than forcing an exact literal match.

Checklist

Please ensure the following tasks are completed before submitting this pull request.

AI Assistance

When authoring the changes proposed in this PR, did you use any kind of AI assistance?

  • Yes
  • No

If you answered "yes" above, how did you use AI assistance?

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Disclosure

If you answered "yes" to using AI assistance, please provide a short disclosure indicating how you used AI assistance. This helps reviewers determine how much scrutiny to apply when reviewing your contribution. Example disclosures: "This PR was written primarily by Claude Code." or "I consulted ChatGPT to understand the codebase, but the proposed changes were fully authored manually by myself.".

This PR was written primarily by Claude Code, investigating a GitHub Actions run failure and proposing a minimal source fix.


@stdlib-js/reviewers


Generated by Claude Code

… <16

The job `linux_test` (Node.js v12, v14) has failed on develop every
scheduled run for the past month with:

  ReferenceError: atob is not defined
      at .../lib/node_modules/@stdlib/string/base/atob/lib/global.js:23

Root cause: `lib/global.js` exported a bare reference to the global
`atob` identifier. Under `'use strict'`, referencing an undeclared
identifier throws a `ReferenceError` at evaluation time, and Node.js
only added `atob`/`btoa` as globals in v16.0.0. Because `lib/index.js`
unconditionally requires `./main.js` (which requires `./global.js`)
before ever checking `hasAtobSupport()`, simply requiring
`@stdlib/string/base/atob` crashed on any Node <16, defeating the
package's own polyfill fallback. The same unconditional require in
`test/test.main.js` crashed at require-time regardless of that test
file's `skip: !hasAtobSupport()` option, since `skip` only gates the
tape assertions, not the top-level `require`.

This commit guards the reference with `typeof atob === 'function'`,
mirroring the identical pattern already used by this package's own
dependency, `@stdlib/assert/has-atob-support/lib/atob.js`. `typeof`
on an undeclared identifier evaluates to `'undefined'` rather than
throwing, so the module now loads safely on Node <16 and correctly
falls back to the polyfill, while behavior on Node >=16 (where the
native `atob` is resolved) is unchanged.

Ref: https://github.com/stdlib-js/stdlib/actions/runs/29241732795
@stdlib-bot

stdlib-bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
string/base/atob $\\color{green}217/217$
$\\color{green}+0.00\\%$
$\\color{red}11/12$
$\\color{green}+0.00\\%$
$\\color{green}2/2$
$\\color{green}+0.00\\%$
$\\color{green}217/217$
$\\color{green}+0.00\\%$

The above coverage report was generated for the changes in this PR.

…bal.js`

CI's `Lint Changed Files` check failed on this branch: the previous
commit's `// eslint-disable-line stdlib/no-redeclare` was an unused
directive (that rule never fires on this line, since `main` is
declared, not `atob`), and the actual violation reported was
`n/no-unsupported-features/node-builtins`, since `atob` is flagged as
unsupported for the package's configured engines range (>=0.10.0).

This commit points the disable comment at the correct rule and
switches the fallback value from `void 0` to `null` to fully match
the precedent in `@stdlib/assert/has-atob-support/lib/atob.js`.
Behavior is unchanged; re-verified manually with and without a global
`atob` present.
@kgryte

kgryte commented Jul 13, 2026

Copy link
Copy Markdown
Member

I believe this is a duplicate PR.

Copy link
Copy Markdown
Member Author

Confirmed — #13329 already proposes the identical fix (same file, same typeof atob === 'function' guard). Closing this in favor of that one.


Generated by Claude Code

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.

4 participants