fix: simplify debug package resolution#10374
Conversation
🦋 Changeset detectedLatest commit: 364995d The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
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 |
create-cloudflare
@cloudflare/kv-asset-handler
miniflare
@cloudflare/pages-shared
@cloudflare/unenv-preset
@cloudflare/vite-plugin
@cloudflare/vitest-pool-workers
@cloudflare/workers-editor-shared
wrangler
commit: |
a33b6ef to
84848e2
Compare
There was a problem hiding this comment.
I have moved these tests to the nodejs hybird app as it requires a different combination of compat date + nodejs_compat flag which doesn't quite fit the setup here.
There was a problem hiding this comment.
What is the difference? I couldn't tell from looking at the Wrangler config files
There was a problem hiding this comment.
The wrangler.json next to the worker is actually irrelevant. The actual test setup is in packages/wrangler/e2e/unenv-preset/preset.test.ts that runs the worker script through a few different combinations of date / flags. But the debug package requires process.env to work properly which needs a compatibility_date after 2025-04-01 and I couldn't figure out a good way to fit this test in 😅
There was a problem hiding this comment.
We don't use the custom build for the debug patch anymore so the build script is now simplified.
84848e2 to
f6d7626
Compare
This comment was marked as outdated.
This comment was marked as outdated.
petebacondarwin
left a comment
There was a problem hiding this comment.
I think this is OK except possibly for the Vite integration but perhaps that can be in a followup if there are things to do there?
| alias: { | ||
| // Force esbuild to use the node implementation of debug instead of unenv's no-op stub. | ||
| // The alias is processed by handleUnenvAliasedPackages which uses require.resolve(). | ||
| debug: "debug", | ||
| }, |
There was a problem hiding this comment.
Turns out the vite plugin resolve the node implementation of debug correctly likely because it defines the module condition before browser. I have confirmed it manually as well.
Vite conditions: ["workerd", "worker", "module", "browser"]
Wrangler conditions: ["workerd", "worker", "browser"]
There was a problem hiding this comment.
Can we add a test?
| }; | ||
|
|
||
| // Test different namespaces based on DEBUG env var: "example:*,test" | ||
| const testNamespace = debug("test"); // Should log (matches "test") |
There was a problem hiding this comment.
I was thrown by these being called "namespaces", when in my mind they are just debugLog functions...
There was a problem hiding this comment.
What is the difference? I couldn't tell from looking at the Wrangler config files
Fixes n/a.