Skip to content

[create-cloudflare] Add Node.js version check before loading CLI#13243

Open
petebacondarwin wants to merge 2 commits intomainfrom
devin/1775122077-c3-node-version-check
Open

[create-cloudflare] Add Node.js version check before loading CLI#13243
petebacondarwin wants to merge 2 commits intomainfrom
devin/1775122077-c3-node-version-check

Conversation

@petebacondarwin
Copy link
Copy Markdown
Contributor

@petebacondarwin petebacondarwin commented Apr 2, 2026

Fixes #13237.

Adds a lightweight bin shim (bin/c3.js) that checks the Node.js version before loading the bundled CLI. Previously, running create-cloudflare on an older Node.js version (e.g. v18) would fail with a confusing syntax error. Now users get a clear message explaining the minimum version and suggesting version managers (Volta/nvm).

This mirrors the existing pattern used by wrangler's bin/wrangler.js.

Changes:

  • New bin/c3.js — plain CommonJS bin shim with embedded semiver for version comparison; reads the minimum version from package.json engines.node so there's a single source of truth
  • package.json: bin now points to ./bin/c3.js instead of ./dist/cli.js; bin dir added to files; engines.node bumped from >=18.14.1 to >=20.0.0
  • Updated AGENTS.md to reflect the new bin shim architecture
  • New test that spawns a child process with a mocked old Node version to verify the error message

Human review checklist

  • The engines.node parsing (replace(">=", "")) assumes the format is always >=X.Y.Z — if the format ever changes (e.g. ^20 or >=20 <22), the bin shim would need updating
  • The engines.node bump from >=18.14.1>=20.0.0 codifies what was already true in practice (the bundled code doesn't run on Node 18), but is technically a breaking change for the engines field
  • The bin shim require("../dist/cli.js") relies on the bundled output auto-executing main(process.argv) at module load — programmatic consumers via main/exports (still pointing at dist/cli.js) bypass the version check, which is intentional

  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: this is an internal DX improvement with no user-facing docs impact

Link to Devin session: https://app.devin.ai/sessions/fa8fa0b8995c42c1a0cb41d27f7de3d6
Requested by: @petebacondarwin


Open with Devin

Add a bin shim (bin/c3.js) that checks the Node.js version before
requiring the bundled CLI. This gives users a clear error message
when running on unsupported Node.js versions instead of a cryptic
syntax error.

Also updates the engines field to require Node.js >= 20.0.0.

Co-Authored-By: pbacondarwin <pete@bacondarwin.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 2, 2026

🦋 Changeset detected

Latest commit: eb534ac

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Apr 2, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2026

✅ All changesets look good

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Apr 2, 2026

Now let me also verify how dist/cli.js is used programmatically via main/exports in package.json, since those still point to ./dist/cli.js — meaning anyone doing require("create-cloudflare") will get the auto-executing module.

Let me also check whether the changeset should be minor instead of patch given the engines.node bump is technically a breaking change.

Now let me check the second test — if dist/cli.js doesn't exist, the require in c3.js will throw a MODULE_NOT_FOUND error. The test expects that in the "happy path" case it does NOT contain the version error in stderr, but the test doesn't actually assert success. Let me verify this is correct.

Let me also check if there's a module === require.main guard concern. Since the bin/c3.js does require("../dist/cli.js"), and dist/cli.js unconditionally calls main(process.argv) at module load time, this will work fine when c3.js is the entry point. But the PR description's concern about double-execution is valid to think about — however since c3.js doesn't call any function from cli.js explicitly, it's just the require triggering the auto-execute, which is the intended behavior.

Now let me think about the actual issues:

  1. The main and exports fields still point to ./dist/cli.js — This means require("create-cloudflare") still imports the auto-executing module directly without the version check. This is fine for programmatic usage (since the calling code already has the right Node version), but it's worth noting.

  2. The changeset says patch but the engines.node bump from >=18.14.1 to >=20.0.0 is a semver concern. The changeset rules say minor is for "breaking changes to experimental/beta/pre-1.0 features" and the package has "prerelease": true in workers-sdk config. Since C3 is marked as prerelease, patch might be acceptable, but bumping engines is typically at least a minor.

  3. Test issue: The second test has a logic gap. If dist/cli.js exists (e.g., from a previous build), the test will actually launch the full C3 CLI with a 5-second timeout and potentially produce flaky results. If dist/cli.js doesn't exist, require() will throw MODULE_NOT_FOUND and exit non-zero, but the test only checks stderr doesn't contain the version message — it doesn't verify the exit code or what happened. The test structure seems fragile.

Actually, wait — the second test catches the error but doesn't fail if execFileSync succeeds (no assertion on success path). If the CLI actually starts and hangs, it'll time out after 5s and throw, which would be caught. This is acceptable but not ideal.

Let me focus on the most actionable issue: the changeset version type.

Actually, re-reading the changeset rules more carefully — the engines.node bump from 18.14.1 to 20.0.0 is indeed what was already true in practice (the bundled code uses syntax that doesn't work on Node 18). The changeset description accurately describes this as making the error message better, not as introducing a new restriction. So patch is arguably correct — it's a bug fix (better error message), and the engines bump just documents reality.

Let me reconsider if there are any actual bugs:

  1. The bin/c3.js uses require("../dist/cli.js") in the same process while wrangler uses spawn in a child process. The key difference: wrangler adds --no-warnings and --experimental-vm-modules flags. If C3 doesn't need those, require is simpler and fine. But there's a subtle issue: process.exitCode = 1 in the version check followed by return — this works correctly because the require is never reached.

  2. The test overrides process.versions.node BEFORE requiring the bin file — this correctly simulates the old version scenario. But the test uses execFileSync which runs in a child process, so the Object.defineProperty inside --eval script correctly affects the child process's process.versions.node before the require loads and runs the version check. This is correct.

I think this PR is actually clean. Let me look one more time at whether there's anything actionable...

One issue: the main export in package.json ("main": "./dist/cli.js") and exports field still point to dist/cli.js. When someone does import { main } from "create-cloudflare", they bypass the version check AND trigger the auto-executing main(process.argv). This was already the case before this PR though (the bin field also pointed to dist/cli.js), so it's not a regression.

Actually, I realize this PR is well-done and doesn't have actionable issues. The approach is sound, mirrors wrangler's pattern, the test is reasonable, and the changeset is appropriate.

LGTM

github run

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Apr 2, 2026

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13243

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13243

miniflare

npm i https://pkg.pr.new/miniflare@13243

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13243

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13243

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13243

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13243

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13243

wrangler

npm i https://pkg.pr.new/wrangler@13243

commit: eb534ac

@petebacondarwin petebacondarwin marked this pull request as ready for review April 2, 2026 10:30
@workers-devprod workers-devprod requested review from a team and NuroDev and removed request for a team April 2, 2026 10:31
@workers-devprod
Copy link
Copy Markdown
Contributor

workers-devprod commented Apr 2, 2026

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

⚠️ 1 issue in files not directly in the diff

⚠️ C3 AGENTS.md not updated to reflect new bin shim architecture (packages/create-cloudflare/AGENTS.md:16-17)

The PR introduces a separate bin/c3.js shim and changes package.json's bin field from ./dist/cli.js to ./bin/c3.js, but packages/create-cloudflare/AGENTS.md:16-17 still states:

  • "package.json main, exports["."], and bin all point at dist/cli.js"
  • "No separate bin shim — the built output IS the bin"

Both statements are now factually incorrect. AI agents relying on AGENTS.md for project context will receive wrong information about the package's entry point architecture, potentially making incorrect changes (e.g., removing the bin shim or re-pointing bin back to dist/cli.js).

View 3 additional findings in Devin Review.

Open in Devin Review

@@ -0,0 +1,51 @@
#!/usr/bin/env node

const MIN_NODE_VERSION = "20.0.0";
Copy link
Copy Markdown
Member

@NuroDev NuroDev Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Would it be better if this was imported from the package.json instead? That way there is a single source for the minimum Node.js version.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call - let's try...

Copy link
Copy Markdown
Contributor

@workers-devprod workers-devprod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codeowners reviews satisfied

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Apr 2, 2026
…e AGENTS.md

- bin/c3.js now reads the minimum Node.js version from package.json
  engines.node instead of hardcoding it
- Updated AGENTS.md to reflect the new bin shim architecture
- Updated test to also read expected version from package.json

Co-Authored-By: pbacondarwin <pete@bacondarwin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Approved

Development

Successfully merging this pull request may close these issues.

create cloudflare fails with bad error on old versions of Node.js

3 participants