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

Revert "feat: add support for custom unenv resolve path" #7583

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

penalosa
Copy link
Contributor

Reverts #7522

This should resolve #7577, and fix the C3 E2E failures we're seeing, which seem to be due to a failure to resolve unenv properly

@penalosa penalosa requested a review from a team as a code owner December 18, 2024 12:36
Copy link

changeset-bot bot commented Dec 18, 2024

🦋 Changeset detected

Latest commit: 8a8178a

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

@penalosa penalosa added the skip-pr-description-validation Skip validation of the required PR description format label Dec 18, 2024
Copy link
Contributor

github-actions bot commented Dec 18, 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/12395886295/npm-package-wrangler-7583

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

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

Or you can use npx with this latest build directly:

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

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.

@penalosa penalosa requested a review from a team as a code owner December 18, 2024 14:16
@penalosa penalosa force-pushed the revert-7522-unenv-preset-path branch from df2b960 to 8a8178a Compare December 18, 2024 15:15
@CarmenPopoviciu
Copy link
Contributor

@penalosa and I have both tested with this pre-release version and can confirm the revert fixes the issue. Will go ahead and merge

@CarmenPopoviciu CarmenPopoviciu merged commit 8def8c9 into main Dec 18, 2024
29 checks passed
@CarmenPopoviciu CarmenPopoviciu deleted the revert-7522-unenv-preset-path branch December 18, 2024 16:00
@vicb
Copy link
Contributor

vicb commented Dec 18, 2024

Many thanks @penalosa for handling the revert of something I broke :(

FYI @pi0 has fixed the root cause in unenv - it is linked to package resolution and the fact that pnpm set the name of "unenv": "npm:unenv-nightly@..." to unenv-nightly in the lock file.

I'll resubmit the PRs and make sure I test the pre-release version locally.

🙏

@petebacondarwin
Copy link
Contributor

I think we need to go further and add tests (probably to nodejs-hybrid fixture) that will prevent this regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-pr-description-validation Skip validation of the required PR description format
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

🐛 BUG: Wrangler pages fails to deploy
4 participants