Skip to content

Minimize the number of the Miniflare package dependencies#11897

Merged
dario-piotrowicz merged 3 commits intomainfrom
dario/DEVX-1578/pin-nd-bundle-miniflare-deps
Jan 23, 2026
Merged

Minimize the number of the Miniflare package dependencies#11897
dario-piotrowicz merged 3 commits intomainfrom
dario/DEVX-1578/pin-nd-bundle-miniflare-deps

Conversation

@dario-piotrowicz
Copy link
Copy Markdown
Member

@dario-piotrowicz dario-piotrowicz commented Jan 13, 2026 •
edited by devin-ai-integration bot
Loading

Fixes https://jira.cfdata.org/browse/DEVX-1578

This PR moves almost all the Miniflare dependencies to the devDependencies field ensuring that these get bundled in.

The non-bunbled packages are all pinned (which also prevents potential future npm vuln issues).

  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
      • I've built Miniflare locally before and after and ensured that in one case the packages were normal dependencies while in the other their code was being bundled in
    • Additional testing not necessary because: Tests/linting for this is going to be added separately: DEVX-1580
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: this change improves the security of the package but is not something that users necessarily need to be aware of

A picture of a cute animal (not mandatory, but encouraged)

Open with Devin

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jan 13, 2026 •
edited
Loading

🦋 Changeset detected

Latest commit: 85a5041

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

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 Jan 13, 2026
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Jan 13, 2026 •
edited
Loading

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@11897

wrangler

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

commit: 5bb6908

@vicb
Copy link
Copy Markdown
Contributor

vicb commented Jan 13, 2026

The non-bunbled packages are all pinned (which also prevents potential future npm vuln issues).

I first think that was a change in your PR - I don't think it is, more an intent (but no related changes) right?

@dario-piotrowicz dario-piotrowicz force-pushed the dario/DEVX-1578/pin-nd-bundle-miniflare-deps branch from 276c611 to 820b018 Compare January 13, 2026 17:05
@dario-piotrowicz
Copy link
Copy Markdown
Member Author

dario-piotrowicz commented Jan 13, 2026

The non-bunbled packages are all pinned (which also prevents potential future npm vuln issues).

I first think that was a change in your PR - I don't think it is, more an intent (but no related changes) right?

Yes, sorry for the confusion, in that sentence I was describing the state of things more than what this PR is doing.

@dario-piotrowicz dario-piotrowicz force-pushed the dario/DEVX-1578/pin-nd-bundle-miniflare-deps branch 6 times, most recently from 7c95626 to 8464d64 Compare January 14, 2026 22:56
@dario-piotrowicz
Copy link
Copy Markdown
Member Author

dario-piotrowicz commented Jan 14, 2026

@petebacondarwin This PR is moving some of the external dependencies to devDependencies, I did that before you merged your PR and everything seemed ok and all the miniflare tests passed

But now the CI check you added errors, and I can see explanations from you here as to why we'd want to avoid bundling those deps?

workers-sdk/packages/miniflare/scripts/deps.ts

Line 7 in fe4faa3

export const EXTERNAL_DEPENDENCIES = [

Based on the tests passing I thought that it'd be ok, but there might be subtle untested consequences? If you are confident that these deps do need to be external I'm happy to close this PR, otherwise we can give this a go, please let me know 🙂

@dario-piotrowicz dario-piotrowicz force-pushed the dario/DEVX-1578/pin-nd-bundle-miniflare-deps branch from 8464d64 to a6eede3 Compare January 15, 2026 12:18
devin-ai-integration[bot]
devin-ai-integration bot reviewed Jan 21, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional flags.

Open in Devin Review

@petebacondarwin petebacondarwin requested a review from a team as a code owner January 21, 2026 16:01
@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Jan 21, 2026
dario-piotrowicz and others added 2 commits January 21, 2026 21:14
@dario-piotrowicz @petebacondarwin
Minimize the number of the Miniflare package dependencies
fab124c
@petebacondarwin
Bundle zod dependency in miniflare and vitest-pool-workers
5bb6908 
Refine the approach to bundling dependencies: only bundle zod (pure JS
library with no native dependencies) while keeping other dependencies
external for technical reasons:

- sharp: Native binary with platform-specific builds
- undici: Dynamically required at runtime in worker threads (fetch-sync.ts)
- ws: Has optional native bindings for performance optimization
- workerd: Native binary (Cloudflare's JS runtime)
- @cspotcode/source-map-support: Uses require.cache manipulation
- youch: Dynamically required for lazy loading

Also updated deps.ts comments to accurately document why each
dependency must remain external.
@petebacondarwin petebacondarwin force-pushed the dario/DEVX-1578/pin-nd-bundle-miniflare-deps branch from aad69fc to 5bb6908 Compare January 21, 2026 21:15
devin-ai-integration[bot]
devin-ai-integration bot reviewed Jan 21, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View issue and 4 additional flags in Devin Review.

Open in Devin Review

@dario-piotrowicz dario-piotrowicz merged commit bbd8a5e into main Jan 23, 2026
41 of 42 checks passed
@dario-piotrowicz dario-piotrowicz deleted the dario/DEVX-1578/pin-nd-bundle-miniflare-deps branch January 23, 2026 09:36
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Jan 23, 2026
@workers-devprod workers-devprod mentioned this pull request Jan 23, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devin-ai-integration devin-ai-integration[bot] devin-ai-integration[bot] left review comments

@petebacondarwin petebacondarwin petebacondarwin approved these changes

Assignees

No one assigned

Labels

None yet

Projects

workers-sdk
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dario-piotrowicz @vicb @petebacondarwin @workers-devprod