Skip to content

Migrate from unbuild to obuild#9243

Merged
CarmenPopoviciu merged 1 commit intomainfrom
vicb/obuild
Jun 18, 2025
Merged

Migrate from unbuild to obuild#9243
CarmenPopoviciu merged 1 commit intomainfrom
vicb/obuild

Conversation

@vicb
Copy link
Contributor

@vicb vicb commented May 14, 2025 •
edited
Loading

unenv now uses obuild (see unjs/unenv#504)

This PR migrates the cf preset to use obuild

/cc @pi0

Some file are missing type exports, i.e. tls is missing:

export declare const checkServerIdentity: typeof import("tls").checkServerIdentity,
	connect: typeof import("tls").connect,
	createSecureContext: typeof import("tls").createSecureContext,
	convertALPNProtocols: any,
	SecureContext: any,
	TLSSocket: typeof import("tls").TLSSocket;

Pooya's comment:

Dts generation issue might come from the new isolated decl generator (we need more explicit exports)
But if is a bug we can also check with kevin (dts plugin maintainer)

I'll investigate This is indeed related to isolated declaration. Should be fine as this is already the way unenv is since it migrated to obuild.

Todo

  • Tests
    • TODO (before merge)
    • Tests included
    • Tests not necessary because: covered by existing tests
  • Wrangler / Vite 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
  • Wrangler V3 Backport
    • TODO (before merge)
    • Wrangler PR:
    • Not necessary because: not a wrangler change

@changeset-bot
Copy link

changeset-bot bot commented May 14, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 4cdce3a

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

This PR includes changesets to release 1 package
Name Type
@cloudflare/unenv-preset 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 May 14, 2025
@vicb vicb added the e2e Run wrangler + vite-plugin e2e tests on a PR label May 14, 2025
@pkg-pr-new
Copy link

pkg-pr-new bot commented Jun 15, 2025 •
edited
Loading

create-cloudflare

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

@cloudflare/kv-asset-handler

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

miniflare

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

@cloudflare/pages-shared

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

@cloudflare/unenv-preset

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

@cloudflare/vite-plugin

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

@cloudflare/vitest-pool-workers

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

@cloudflare/workers-editor-shared

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

wrangler

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

commit: 4cdce3a

@vicb vicb marked this pull request as ready for review June 18, 2025 07:54
@vicb vicb requested a review from a team as a code owner June 18, 2025 07:54
@vicb vicb requested a review from a team June 18, 2025 07:54
petebacondarwin
petebacondarwin approved these changes Jun 18, 2025
View reviewed changes
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

LGTM - out of interest, why are we updating to this experimental tool?

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Jun 18, 2025
@vicb
Copy link
Contributor Author

vicb commented Jun 18, 2025

LGTM - out of interest, why are we updating to this experimental tool?

unbuild is being replaced by obuild so probably needed at some point.
and it's what unenv uses now (see PR description for ref)

@CarmenPopoviciu CarmenPopoviciu merged commit 251bad3 into main Jun 18, 2025
22 of 24 checks passed
@CarmenPopoviciu CarmenPopoviciu deleted the vicb/obuild branch June 18, 2025 16:27
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Jun 18, 2025
@workers-devprod workers-devprod mentioned this pull request Jun 18, 2025
Merged
@vicb
Copy link
Contributor Author

vicb commented Jun 18, 2025

This breaks windows CI:

.\dist\runtime\polyfill\performance.mjs
file:///D:/a/workers-sdk/workers-sdk/node_modules/.pnpm/rolldown@1.0.0-beta.9/node_modules/rolldown/dist/shared/src-PO2pehsE.mjs:2247
	const wrapper = new Error(summary);
	                ^

Error: Build failed with 1 error:

[PARSE_ERROR] Error: Invalid escape sequence
   ╭─[ #entry:1:61 ]
   │
 1 │ import * as _lib from "D:\a\workers-sdk\workers-sdk\packages\unenv-preset\dist\index.mjs";
   │                                                             ─┬  
   │                                                              ╰── 
───╯

FYI @pi0

We will revert and investigate

jseba added a commit to jseba/workers-sdk that referenced this pull request Jun 18, 2025
@jseba
Merge branch 'main' of ssh://github.com/cloudflare/workers-sdk into j…
d4f7f8b 
…seba/containers_scope

* 'main' of ssh://github.com/cloudflare/workers-sdk: (31 commits)
  Refactor preview mode and ensure compatibility with Vite 7 (cloudflare#9647)
  Block requests vulnerable to opennext vulnerability (cloudflare#9635)
  Add test for cloudchamber buildAndMaybePush (cloudflare#9638)
  chore: remove redundant binding guide superseded by internal docs (cloudflare#9648)
  add changeset to trigger release of workers/pages projects (cloudflare#9649)
  Add @handler to Python templates. (cloudflare#9305)
  Migrate from unbuild to obuild (cloudflare#9243)
  Version Packages (cloudflare#9650)
  fix changeset (cloudflare#9651)
  containers: Default scheduling policy should be the default (cloudflare#9621)
  Rename Mixed Mode to remote proxy/remote bindings depending on context (cloudflare#9586)
  Version Packages (cloudflare#9632)
  Correctly mock out getDockerImageDigest for testing buildAndMaybePush (cloudflare#9636)
  [C3] Bump create-remix from 2.16.6 to 2.16.8 in /packages/create-cloudflare/src/frameworks (cloudflare#9525)
  Remove "Cloudchamber" from user facing error messages (cloudflare#9628)
  sync local containers with latest workerd (cloudflare#9576)
  Bump the workerd-and-workers-types group with 2 updates (cloudflare#9591)
  [C3] Bump gatsby from 5.14.3 to 5.14.4 in /packages/create-cloudflare/src/frameworks (cloudflare#9524)
  [C3] Bump create-react-router from 7.6.1 to 7.6.2 in /packages/create-cloudflare/src/frameworks (cloudflare#9526)
  [C3] Bump create-docusaurus from 3.8.0 to 3.8.1 in /packages/create-cloudflare/src/frameworks (cloudflare#9527)
  ...
@petebacondarwin
Copy link
Contributor

petebacondarwin commented Jun 18, 2025

This is a classic case of injecting paths into code as a string. There is probably something that looks like:

const code = `import * as _lib from "${filePath}";`;

which works fine on Unix but with Windows paths the \ characters become escape sequences.

Instead you need something like:

const code = `import * as _lib from ${JSON.stringify(filePath)};`;

CarmenPopoviciu added a commit that referenced this pull request Jun 19, 2025
@CarmenPopoviciu
Revert "Migrate from unbuild to obuild (#9243)"
40e2a71 
This reverts commit 251bad3.
@CarmenPopoviciu CarmenPopoviciu mentioned this pull request Jun 19, 2025
Merged
CarmenPopoviciu added a commit that referenced this pull request Jun 19, 2025
@CarmenPopoviciu
Revert "Migrate from unbuild to obuild (#9243)" (#9669)
3d32a4e 
This reverts commit 251bad3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petebacondarwin petebacondarwin petebacondarwin approved these changes

Assignees

No one assigned

Labels

e2e Run wrangler + vite-plugin e2e tests on a PR

Projects

workers-sdk
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vicb @petebacondarwin @CarmenPopoviciu