Skip to content

fix(@stdlib/string/base/atob): guard against ReferenceError when atob global is absent - #14592

Draft
Planeshifter wants to merge 2 commits into
developfrom
claude/elegant-feynman-dit39q
Draft

fix(@stdlib/string/base/atob): guard against ReferenceError when atob global is absent#14592
Planeshifter wants to merge 2 commits into
developfrom
claude/elegant-feynman-dit39q

Conversation

@Planeshifter

Copy link
Copy Markdown
Member

Description

What is the purpose of this pull request?

This pull request:

  • Fixes lib/node_modules/@stdlib/string/base/atob/lib/global.js: module.exports = atob; referenced the bare atob global unconditionally. Node.js only defines atob globally starting in v16; on Node.js 12/14, evaluating the undeclared identifier threw a ReferenceError at module load, not at call time. Since lib/main.js requires this module eagerly, and the package's test files require lib/main.js at top level, the crash happened before the existing hasAtobSupport()-gated tape skip logic could run, breaking the linux_test workflow's Node.js v12 and v14 jobs on every scheduled run (https://github.com/stdlib-js/stdlib/actions/runs/32710852762). Fix: guard the reference with var main = ( typeof atob === 'function' ) ? atob : null;, matching the existing idiom in @stdlib/assert/has-atob-support/lib/atob.js. require('@stdlib/string/base/atob') in production is unaffected — that path resolves through lib/index.js via hasAtobSupport() and never hits this file's bug.

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.

Diff: one file, 6 lines added, 1 removed.

No node_modules install was possible in this environment, so verification used Node's vm module to evaluate the file in a fresh V8 context: no throw and correct null fallback with atob absent from globals, correct pass-through with atob present.

Reviewed independently across correctness, regression scope, and style/conventions before opening. All three passes flagged the same non-blocking issue from an early draft: an // eslint-disable-line stdlib/no-redeclare comment was unnecessary — that rule only fires on declarations shadowing a global of the same name, and var main doesn't shadow atob — and would have tripped --report-unused-disable-directives. Removed before commit.

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 investigated, written, and validated by Claude Code as part of an automated CI-failure triage routine: it identified the failing linux_test jobs, traced the root cause, implemented the fix, and validated it via three independent automated review passes (correctness, regression scope, style/conventions) before opening this PR.


@stdlib-js/reviewers


Generated by Claude Code

claude added 2 commits August 24, 2026 14:18
…tob` global is absent

The job `Node.js v12` and `Node.js v14` of workflow `linux_test` failed
on develop with `ReferenceError: atob is not defined`. Root cause:
`lib/global.js` unconditionally referenced the bare global identifier
`atob`, which Node.js only defines starting in v16, so merely
requiring the module threw at evaluation time on older versions. This
module is required eagerly by `lib/main.js`, itself required eagerly
by the package's test files, crashing the test suite before the
existing `hasAtobSupport()` skip logic ever ran. This commit guards
the reference with `typeof atob === 'function'`, matching the existing
pattern in `@stdlib/assert/has-atob-support`, so requiring the module
no longer throws on environments lacking a native `atob` global.

Ref: https://github.com/stdlib-js/stdlib/actions/runs/32710852762
… error

CI's `Lint Changed Files` check failed on the prior commit:
`n/no-unsupported-features/node-builtins` flags any reference to the
`atob` identifier as unsupported given this package's `engines.node`
range (`>=0.10.0`), including inside `typeof atob`. The established
repo convention for this exact situation is an inline
`eslint-disable-line n/no-unsupported-features/node-builtins` comment
(see `@stdlib/process/geteuid/lib/native.js`); this commit adds it.

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

stdlib-bot commented Aug 24, 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.

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.

3 participants