Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(wrangler): update unenv dependency version #7541

Merged
merged 2 commits into from
Dec 17, 2024
Merged

chore(wrangler): update unenv dependency version #7541

merged 2 commits into from
Dec 17, 2024

Conversation

vicb
Copy link
Contributor

@vicb vicb commented Dec 13, 2024

Update unenv to the latest release.

unenv now resolves aliases to absolute path so the plugin does not it to do it any more.

/cc @petebacondarwin @pi0


  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because: tested by existing tests
  • E2E Tests CI Job required? (Use "e2e" label or ask maintainer to run separately)
    • I don't know
    • Required
    • Not required because:
  • Public documentation
    • TODO (before merge)
    • Cloudflare docs PR(s):
    • Documentation not necessary because: no user facing change

This is part 1 of 2 in a stack made with GitButler:

@vicb vicb requested a review from a team as a code owner December 13, 2024 13:21
Copy link

changeset-bot bot commented Dec 13, 2024

🦋 Changeset detected

Latest commit: 7913462

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

This PR includes changesets to release 2 packages
Name Type
wrangler Patch
@cloudflare/vitest-pool-workers Patch

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

@vicb vicb added the e2e Run e2e tests on a PR label Dec 13, 2024
@vicb vicb requested a review from petebacondarwin December 13, 2024 13:22
Copy link
Contributor

github-actions bot commented Dec 13, 2024

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/12367522688/npm-package-wrangler-7541

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

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

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12367522688/npm-package-wrangler-7541 dev path/to/script.js
Additional artifacts:
wget https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12367522688/npm-package-cloudflare-workers-bindings-extension-7541 -O ./cloudflare-workers-bindings-extension.0.0.0-v859e5119b.vsix && code --install-extension ./cloudflare-workers-bindings-extension.0.0.0-v859e5119b.vsix
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12367522688/npm-package-create-cloudflare-7541 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12367522688/npm-package-cloudflare-kv-asset-handler-7541
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12367522688/npm-package-miniflare-7541
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12367522688/npm-package-cloudflare-pages-shared-7541
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12367522688/npm-package-cloudflare-vitest-pool-workers-7541
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12367522688/npm-package-cloudflare-workers-editor-shared-7541
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12367522688/npm-package-cloudflare-workers-shared-7541
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/12367522688/npm-package-cloudflare-workflows-shared-7541

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


[email protected] includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20241205.0
workerd 1.20241205.0 1.20241205.0
workerd --version 1.20241205.0 2024-12-05

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

@@ -10090,7 +10090,7 @@ export default{
}
`);
expect(fs.readFileSync("dist/index.js", { encoding: "utf-8" })).toContain(
`import path from "path";`
`import path from "node:path";`
Copy link
Contributor Author

@vicb vicb Dec 13, 2024

Choose a reason for hiding this comment

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

This change is required because unenv module resolution will always append the node: prefix

@@ -10069,7 +10069,7 @@ export default{
fs.writeFileSync(
"index.js",
`
import path from 'path';
import path from 'node:path';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is not required - the output is the same with or without the prefix.
Added only for consistency with l10093 below

// Convert `require()` calls for NPM packages to a virtual ES Module that can be imported avoiding the require calls.
// Note: Does not apply to Node.js packages that are handled in `handleRequireCallsToNodeJSBuiltins`
if (
args.kind === "require-call" &&
(unresolvedAlias.startsWith("unenv/runtime/npm/") ||
unresolvedAlias.startsWith("unenv/runtime/mock/"))
(unresolved.startsWith("unenv/runtime/npm/") ||
Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend using resolveAlias from pathe/utils (pathe is safe against windows paths when comparing and also handles multiple recursive aliases a => b => c)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure to understand how it would help here?
"unenv/runtime/npm/" comes from the preset alias.

@petebacondarwin
Copy link
Contributor

unenv now resolves aliases to absolute path so the plugin does not it to do it any more.

🎉

@vicb vicb merged commit ca9410a into main Dec 17, 2024
31 checks passed
@vicb vicb deleted the unenv-up branch December 17, 2024 07:10
penalosa added a commit that referenced this pull request Dec 18, 2024
CarmenPopoviciu pushed a commit that referenced this pull request Dec 18, 2024
* Revert "feat: add support for custom unenv resolve path (#7522)"

This reverts commit 6403e41.

* Create thick-ties-glow.md

* Revert "chore(wrangler): update unenv dependency version (#7541)"

This reverts commit ca9410a.

* fix lockfile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e Run e2e tests on a PR
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants