Skip to content

fix(@stdlib/string/base/atob): guard against undefined atob global#13557

Closed
Planeshifter wants to merge 2 commits into
developfrom
philipp/ci-fix-atob-referenceerror-nodejs12-2026-07-19
Closed

fix(@stdlib/string/base/atob): guard against undefined atob global#13557
Planeshifter wants to merge 2 commits into
developfrom
philipp/ci-fix-atob-referenceerror-nodejs12-2026-07-19

Conversation

@Planeshifter

Copy link
Copy Markdown
Member

Description

What is the purpose of this pull request?

This pull request:

  • Fixes a ReferenceError: atob is not defined crash on Node.js v12/v14 in @stdlib/string/base/atob by guarding the module-scope reference to the atob global.

Related Issues

Does this pull request have any related issues?

This pull request has the following related issues:

  • None.

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
linux_test #29681875444, jobs "Node.js v12" and "Node.js v14", branch develop.

Symptom

ReferenceError: atob is not defined
    at Object.<anonymous> (.../lib/node_modules/@stdlib/string/base/atob/lib/global.js:23:18)

Root cause
global.js did module.exports = atob; — a bare reference evaluated at require time. Node <16 doesn't define atob globally. index.js requires main.js (which requires global.js) unconditionally, before @stdlib/assert/has-atob-support ever runs, so the crash preempts the environment-detection logic meant to handle exactly this case.

Fix
global.js now reads:

var main = ( typeof atob === 'function' ) ? atob : null;
module.exports = main;

typeof on an undeclared identifier never throws. On Node 16+/browsers, behavior is unchanged — the native function is exported as before. hasAtobSupport() still gates the polyfill path in index.js; main.js's try/catch is untouched.

Validation

  • vm sandbox with no global atob: global.js no longer throws, exports null.
  • End-to-end on native-atob Node: atob('SGVsbG8sIHdvcmxk')'Hello, world' through global.jsmain.jsindex.js.
  • npm test / make lint-pkg not runnable in this sandbox (no node_modules; npm install blocked by registry min-release-age policy on es-object-atoms@^1.1.2). node --check confirms valid syntax.
  • Three independent reviews (correctness, regression-scope, style) — all approve, no blocking findings.

Reviewer notes
Non-blocking: fix is platform-agnostic and should also cover this failure class on other Node <16 matrix jobs (windows_test_npm_install, macos_test_npm_install), not just linux_test.

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, running an automated CI-failure investigation and fix routine: it identified the failing run, diagnosed the root cause, authored the fix, and drafted this description, all based on the linked CI failure log.


@stdlib-js/reviewers


Generated by Claude Code

The job `Node.js v12` and `Node.js v14` on workflow `linux_test` failed
on develop with `ReferenceError: atob is not defined` when requiring
`@stdlib/string/base/atob`. Root cause: `lib/global.js` referenced the
bare `atob` identifier at module-evaluation time; Node.js versions
below v16 do not expose `atob` as a global, so `require()` throws
before `lib/index.js` ever gets a chance to route to the polyfill via
`@stdlib/assert/has-atob-support`. This commit guards the reference
with `typeof atob === 'function'`, mirroring the established pattern
used by `@stdlib/assert/has-atob-support/lib/atob.js` and
`@stdlib/utils/global/lib/global.js`, so that requiring the module no
longer throws on environments lacking a native `atob`.

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

stdlib-bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

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

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

…rule

The `Lint Changed Files` check on PR #13557 failed on `global.js` with
`n/no-unsupported-features/node-builtins`: the `eslint-plugin-n` rule
flags the guarded `atob` reference because the package's configured
minimum supported Node.js version is below v16.0.0, when `atob` first
became a global. The reference is intentionally guarded via `typeof`
for exactly this reason, matching the same pattern already used
elsewhere in the codebase for other version-gated builtins, e.g.
`@stdlib/os/homedir/lib/index.js`. This commit adds the matching
`eslint-disable-line` comment so the lint check passes without
changing runtime behavior.

Ref: https://github.com/stdlib-js/stdlib/actions/runs/29690512042
@kgryte

kgryte commented Jul 19, 2026

Copy link
Copy Markdown
Member

Isn't this a duplicate PR?

Copy link
Copy Markdown
Member Author

Yes — this duplicates #13329, opened 2026-07-06 for the same root cause, still open and passing CI. I missed the open-PR check before starting this one. Closing in favor of #13329.


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