Revert "fix(wrangler): handling of process.env.NODE_ENV in bundling mode"

[edmundhung]

Reverts #7932.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 5f55c3d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[github-actions]

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-wrangler-7967

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/7967/npm-package-wrangler-7967

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-wrangler-7967 dev path/to/script.js

Additional artifacts:

cloudflare-workers-bindings-extension:

wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-cloudflare-workers-bindings-extension-7967 -O ./cloudflare-workers-bindings-extension.0.0.0-va3dea7b1b.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-va3dea7b1b.vsix

create-cloudflare:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-create-cloudflare-7967 --no-auto-update

@cloudflare/kv-asset-handler:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-cloudflare-kv-asset-handler-7967

miniflare:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-miniflare-7967

@cloudflare/pages-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-cloudflare-pages-shared-7967

@cloudflare/unenv-preset:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-cloudflare-unenv-preset-7967

@cloudflare/vite-plugin:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-cloudflare-vite-plugin-7967

@cloudflare/vitest-pool-workers:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-cloudflare-vitest-pool-workers-7967

@cloudflare/workers-editor-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-cloudflare-workers-editor-shared-7967

@cloudflare/workers-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-cloudflare-workers-shared-7967

@cloudflare/workflows-shared:

npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/13048240333/npm-package-cloudflare-workflows-shared-7967

Note that these links will no longer work once the GitHub Actions artifact expires.

---

wrangler@3.106.0 includes the following runtime dependencies:

Package	Constraint	Resolved
miniflare	workspace:*	3.20250124.0
workerd	1.20250124.0	1.20250124.0
workerd --version	1.20250124.0	2025-01-24

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

[petebacondarwin]

Can you provide some explanation why this is being reverted?

[lrapoport-cf]

Can you provide some explanation why this is being reverted?

my understanding is that although #7932 indicates in the PR description that it has tests, they're actually missing from that PR. so this revert is to go back until tests can be added. @edmundhung @vicb is this correct?

[vicb]

I think that is the case, my answer to @edmundhung yesterday

You're absolutely right, tests are not included, I definitely should have mentioned that.
There is one more point to be fixed in the original issue (when process is imported rather than using the global) that definitely needs testing.
This PR only includes the fix that are obvious but I can understand if it cannot be merged without tests.

[vicb]

To be clear:

• I think that the current code is broken
• I think that the reverted PR fixes some of the issues
• The current is not tested (properly or at all, that I don't know)
• I think the reverted PR is an improvement
• BUT I fully understand why no test is a reason to revert the PR... i.e. we wouldn't have broken in the first place if we caught that it was not tested.

I think it would be good to add test soon and fixed the remaining point in the original issue

[edmundhung]

I have prepared this PR just in case we are feeling uncomfortable with the changes for today's release. But happy to close this if we have no concern :) Personally I think the change is minimal and it is quite clear what it fixed.

[vicb]

I'll defer to others for the decision.

Whatever the decision, I think #7886 should be prioritized.

[penalosa]

@vicb @edmundhung I think the change is minimal and shouldn't cause issues (if it would, we'd see other tests failing). However, perhaps @vicb you could add a simple test that checks that Wrangler correctly statically defines process.env.NODE_ENV? The navigator define is already tested.

In terms of fixing the imported process, personally I think we should punt on that until the runtime solution lands, since the situation right now is stable.

[vicb]

@vicb @edmundhung I think the change is minimal and shouldn't cause issues (if it would, we'd see other tests failing). However, perhaps @vicb you could add a simple test that checks that Wrangler correctly statically defines process.env.NODE_ENV? The navigator define is already tested.

Could you point me to a starting point (i.e. existing test)?

In terms of fixing the imported process, personally I think we should punt on that until the runtime solution lands, since the situation right now is stable.

Are you thinking of cloudflare/workerd#3311?
If that's the case I think that it should be considered orthogonal.
Quite a few libs out there have debug code guarded by if (process.env.NODE === "development") { ... } (or similar). While having the value at runtime would make sure the code is not executed, it would still be in the bundle (including potential dependencies).
Having process.env.NODE replaced at build time would ensure the code is tree shaken and would allow reducing the bundle size.
(Also 3311 is paused and will be guarded by a compatibility flag when released)

[penalosa]

@vicb the navigator tests are in https://github.com/cloudflare/workers-sdk/blob/ce8bfbdada90d1eac99f3a35dfdf1707e88eb73f/packages/wrangler/src/__tests__/navigator-user-agent.test.ts, which should be a good starting point.

[edmundhung]

We will revert this as agreed and will land this change again once we have the test ready 👍🏼