Skip to content

remove node:module polyfills #10431

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Aug 28, 2025
Merged

remove node:module polyfills #10431

merged 4 commits into from
Aug 28, 2025

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Aug 21, 2025 •
edited by vicb
Loading

Removes node:module polyfills

Depends on cloudflare/workerd#4857, released in v1.20250823.0

  • Tests
    • Tests included
    • Tests not necessary because:
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: Will open a PR in bulk.
  • Wrangler V3 Backport
    • Wrangler PR:
    • Not necessary because: Might be a breaking change

@anonrig anonrig requested review from a team as code owners August 21, 2025 18:20
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Aug 21, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 8cef63a

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

This PR includes changesets to release 4 packages
Name Type
@cloudflare/unenv-preset Patch
@cloudflare/vite-plugin Patch
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

@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Aug 21, 2025
@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Aug 21, 2025 •
edited
Loading

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

wrangler

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

commit: 8cef63a

@github-actions GitHub Actions
Copy link
Contributor

Failed to automatically backport this PR's changes to Wrangler v3. Please manually create a PR targeting the v3-maintenance branch with your changes. Thank you for helping us keep Wrangler v3 supported!

Depending on your changes, running git rebase --onto v3-maintenance main yagiz/remove-node-module-preset might be a good starting point.

Notes:

  • your PR branch should be named v3-backport-10431
  • add the skip-v3-pr label to the current PR to stop this workflow from failing

@anonrig anonrig added the skip-v3-pr Skip validation of presence of a v3 backport PR label Aug 22, 2025
@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Aug 22, 2025
vicb
vicb requested changes Aug 25, 2025
View reviewed changes
Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

Do we need to bump the minimum workerd version (per dep on workerd in package.json)?

@github-project-automation github-project-automation bot moved this from Approved to In Review in workers-sdk Aug 25, 2025
@anonrig anonrig force-pushed the yagiz/remove-node-module-preset branch from c3111d0 to 37ea43c Compare August 26, 2025 15:25
@anonrig anonrig requested a review from vicb August 26, 2025 15:25
@anonrig anonrig force-pushed the yagiz/remove-node-module-preset branch from 37ea43c to c37659d Compare August 26, 2025 22:10
@anonrig
Copy link
Member Author

anonrig commented Aug 26, 2025

Do we need to bump the minimum workerd version (per dep on workerd in package.json)?

I've rebased this branch (hence updated the workerd version). Can you re-review? @vicb

vicb
vicb reviewed Aug 27, 2025
View reviewed changes
vicb
vicb reviewed Aug 27, 2025
View reviewed changes
Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

Depends on cloudflare/workerd#4857

For unenv PR, when there are dependencies, could you please add the workerd release when the changes are available to the PR description.

Thanks!

@anonrig
Copy link
Member Author

anonrig commented Aug 27, 2025

For unenv PR, when there are dependencies, could you please add the workerd release when the changes are available to the PR description.

I'm not following @vicb. Another PR of yours already updated the workerd version. This PR doesn't have to include it since it's already in main branch.

@vicb vicb closed this Aug 28, 2025
@github-project-automation github-project-automation bot moved this from In Review to Done in workers-sdk Aug 28, 2025
@vicb vicb reopened this Aug 28, 2025
@github-project-automation github-project-automation bot moved this from Done to Untriaged in workers-sdk Aug 28, 2025
@vicb
Copy link
Contributor

vicb commented Aug 28, 2025

For unenv PR, when there are dependencies, could you please add the workerd release when the changes are available to the PR description.

I'm not following @vicb. Another PR of yours already updated the workerd version. This PR doesn't have to include it since it's already in main branch.

See how I updated the PR description:

Depends on cloudflare/workerd#4857, released in v1.20250823.0

This PR needs workerd#4857 that has been released in workerd 20250823. That's the piece of info that I need when reviewing the PR.

As of today unenv depends on 20250816 which does not contain the changes

workers-sdk/packages/unenv-preset/package.json

Line 52 in 452ad0b

"workerd": "^1.20250816.0"

When reviewing, If I had that info I could have added: please update the workerd peer dep.

I'll go ahead and amend the PR to update the peer dep for this PR and try to land this today.

But please for other unenv PRs add what the required workerd version is.

Does this make sense?

@vicb vicb force-pushed the yagiz/remove-node-module-preset branch from 09218ea to 8cef63a Compare August 28, 2025 08:41
vicb
vicb approved these changes Aug 28, 2025
View reviewed changes
Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

I have updated the peer dep to 20250823 and rebased.

The PR is ready to go

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Aug 28, 2025
@vicb vicb merged commit f656d1a into main Aug 28, 2025
47 of 50 checks passed
@vicb vicb deleted the yagiz/remove-node-module-preset branch August 28, 2025 12:02
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Aug 28, 2025
@workers-devprod workers-devprod mentioned this pull request Aug 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vicb vicb vicb approved these changes

@jasnell jasnell jasnell approved these changes

@dario-piotrowicz dario-piotrowicz dario-piotrowicz approved these changes

@petebacondarwin petebacondarwin Awaiting requested review from petebacondarwin

Assignees
No one assigned
Labels
skip-v3-pr Skip validation of presence of a v3 backport PR
Projects
workers-sdk
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@anonrig @vicb @jasnell @dario-piotrowicz