Skip to content

Conversation

@shbhmexe
Copy link

PR description

This PR delivers a focused set of safe improvements that keep existing behavior intact while addressing a real bug and two quality issues:

  1. Fix build-content-tree script
  • Problem: fs.writeFileSync was called with a callback, which is invalid for the synchronous API and throws a TypeError. This breaks npm run content and any CI job invoking the script.
  • Fix: Replace the callback with a try/catch and explicit logging. On success the file is written as before; on error we log and set process.exitCode = 1.
  1. Harden is-client detection and unify module style
  • Problem: Directly referencing window at module scope can throw ReferenceError in non‑browser environments (SSR/SSG, tests, or tooling). The file also used CJS export while the rest of the codebase predominantly uses ESM imports.
  • Fix: Guard with typeof window !== 'undefined' && typeof window.document !== 'undefined' and export a default ESM value. All existing imports already use import isClient from ..., so this is backward‑compatible.
  1. Clarify content tree flattener
  • Problem: Array.map was used purely for side effects (the returned array was ignored) in flatten-content-tree.mjs.
  • Fix: Replace map with forEach and add a small defensive Array.isArray check. This improves readability and avoids lint noise without changing behavior.

Rationale and impact

  • Stability: Prevents an immediate runtime error in the content build script.
  • Correctness: Ensures safe execution in non‑browser contexts and proper script exit signaling.
  • Readability: Clarifies iteration intent.
  • Scope: 3 files touched; no new dependencies; no public API or feature changes.

Testing and verification

  • Script correctness: Running node src/scripts/build-content-tree.mjs ./src/content ./src/_content.json will now succeed (assuming deps installed) and log success; failures set a non‑zero exit code.
  • App behavior: isClient remains true in browsers and false elsewhere. Components importing it (Site, Splash, index.jsx) continue to work unchanged.
  • Build: No config changes. Webpack/SSG paths are unaffected.

Backward compatibility

  • No public API changes. Only internal implementation details were updated.
  • The ESM default export for is-client matches current import sites; there are no require() callers in the repository.

…minor cleanup

- src/scripts/build-content-tree.mjs: replace incorrect fs.writeFileSync callback
  with try/catch. The synchronous API does not accept a callback; passing one
  throws a TypeError and breaks `npm run content`. Now the script logs success
  and sets a non‑zero exit code on failure.
- src/utilities/is-client.js: avoid ReferenceError in non‑browser contexts by
  guarding with `typeof window !== 'undefined'` and switch to a consistent ESM
  default export. This prevents crashes during SSR/SSG or tooling that evaluates
  the module outside the browser.
- src/utilities/flatten-content-tree.mjs: use `forEach` instead of `map` when
  not using the returned array. This clarifies intent and avoids linter warnings
  about the unused result of `map`.

These changes are internal, do not alter runtime behavior of the website, and
improve correctness, stability, and readability.

Signed-off-by: shubham shukla <shubhushukla586@gmail.com>
@vercel
Copy link

vercel bot commented Nov 24, 2025

@shbhmexe is attempting to deploy a commit to the OpenJS Foundation Team on Vercel.

A member of the Team first needs to authorize it.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 24, 2025

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: shbhmexe / name: Shubham shukla (98ce638)

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