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.

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

Revert "chore: externalize undici for bundling" #65243

Closed
wants to merge 3 commits into from
Closed

Revert "chore: externalize undici for bundling" #65243

wants to merge 3 commits into from

Conversation

samcx
Copy link
Member

@samcx samcx commented May 1, 2024 •
edited by huozhi

undici should be able to compile and bundle now.

Reverts #64209

Closes NEXT-3274
x-ref: slack

@samcx samcx self-assigned this May 1, 2024
@samcx samcx requested review from ismaelrumzan and leerob and removed request for a team May 1, 2024 18:20
@linear Linear
Copy link

linear bot commented May 1, 2024

Issue reopened: NEXT-3030 chore: externalize undici for bundling

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team Documentation Related to Next.js' official documentation. tests type: next labels May 1, 2024
@ijjk
Copy link
Member

ijjk commented May 1, 2024 •
edited

Stats from current PR

Default Build (Increase detected ⚠️)
General
vercel/next.js canary vercel/next.js revert-64209-externalize-udici Change
buildDuration 30.1s 26.2s N/A
buildDurationCached 17s 12.7s N/A
nodeModulesSize 360 MB 360 MB N/A
nextStartRea..uration (ms) 762ms 710ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js revert-64209-externalize-udici Change
1103-HASH.js gzip 31.9 kB 31.9 kB N/A
1a9f679d-HASH.js gzip 53.5 kB 53.5 kB N/A
335-HASH.js gzip 5.09 kB 5.09 kB N/A
7953.HASH.js gzip 169 B 169 B ✓
framework-HASH.js gzip 45.2 kB 45.2 kB ✓
main-app-HASH.js gzip 231 B 230 B N/A
main-HASH.js gzip 31.5 kB 31.5 kB N/A
webpack-HASH.js gzip 1.65 kB 1.65 kB N/A
Overall change 45.4 kB 45.4 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js revert-64209-externalize-udici Change
polyfills-HASH.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary vercel/next.js revert-64209-externalize-udici Change
_app-HASH.js gzip 192 B 193 B N/A
_error-HASH.js gzip 192 B 193 B N/A
amp-HASH.js gzip 507 B 511 B N/A
css-HASH.js gzip 341 B 343 B N/A
dynamic-HASH.js gzip 2.52 kB 2.52 kB ✓
edge-ssr-HASH.js gzip 266 B 265 B N/A
head-HASH.js gzip 362 B 365 B N/A
hooks-HASH.js gzip 392 B 392 B ✓
image-HASH.js gzip 4.32 kB 4.32 kB N/A
index-HASH.js gzip 268 B 268 B ✓
link-HASH.js gzip 2.69 kB 2.7 kB N/A
routerDirect..HASH.js gzip 329 B 328 B N/A
script-HASH.js gzip 397 B 397 B ✓
withRouter-HASH.js gzip 324 B 324 B ✓
1afbb74e6ecf..834.css gzip 106 B 106 B ✓
Overall change 4 kB 4 kB ✓
Client Build Manifests
vercel/next.js canary vercel/next.js revert-64209-externalize-udici Change
_buildManifest.js gzip 483 B 484 B N/A
Overall change 0 B 0 B ✓
Rendered Page Sizes
vercel/next.js canary vercel/next.js revert-64209-externalize-udici Change
index.html gzip 529 B 528 B N/A
link.html gzip 541 B 540 B N/A
withRouter.html gzip 524 B 524 B ✓
Overall change 524 B 524 B ✓
Edge SSR bundle Size
vercel/next.js canary vercel/next.js revert-64209-externalize-udici Change
edge-ssr.js gzip 94.7 kB 94.7 kB N/A
page.js gzip 181 kB 181 kB N/A
Overall change 0 B 0 B ✓
Middleware size
vercel/next.js canary vercel/next.js revert-64209-externalize-udici Change
middleware-b..fest.js gzip 622 B 623 B N/A
middleware-r..fest.js gzip 156 B 156 B ✓
middleware.js gzip 25.7 kB 25.7 kB N/A
edge-runtime..pack.js gzip 839 B 839 B ✓
Overall change 995 B 995 B ✓
Next Runtimes
vercel/next.js canary vercel/next.js revert-64209-externalize-udici Change
app-page-exp...dev.js gzip 171 kB 171 kB ✓
app-page-exp..prod.js gzip 98.5 kB 98.5 kB ✓
app-page-tur..prod.js gzip 100 kB 100 kB ✓
app-page-tur..prod.js gzip 94.4 kB 94.4 kB ✓
app-page.run...dev.js gzip 157 kB 157 kB ✓
app-page.run..prod.js gzip 93.1 kB 93.1 kB ✓
app-route-ex...dev.js gzip 21.5 kB 21.5 kB ✓
app-route-ex..prod.js gzip 15.2 kB 15.2 kB ✓
app-route-tu..prod.js gzip 15.2 kB 15.2 kB ✓
app-route-tu..prod.js gzip 15 kB 15 kB ✓
app-route.ru...dev.js gzip 21.3 kB 21.3 kB ✓
app-route.ru..prod.js gzip 15 kB 15 kB ✓
pages-api-tu..prod.js gzip 9.55 kB 9.55 kB ✓
pages-api.ru...dev.js gzip 9.82 kB 9.82 kB ✓
pages-api.ru..prod.js gzip 9.55 kB 9.55 kB ✓
pages-turbo...prod.js gzip 21.5 kB 21.5 kB ✓
pages.runtim...dev.js gzip 22.1 kB 22.1 kB ✓
pages.runtim..prod.js gzip 21.5 kB 21.5 kB ✓
server.runti..prod.js gzip 51.6 kB 51.6 kB ✓
Overall change 963 kB 963 kB ✓
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js revert-64209-externalize-udici Change
0.pack gzip 1.62 MB 1.62 MB ⚠️ +1.34 kB
index.pack gzip 113 kB 113 kB N/A
Overall change 1.62 MB 1.62 MB ⚠️ +1.34 kB
Diff details
Diff for edge-ssr.js

Diff too large to display

Commit: 40ce1bf

huozhi
huozhi previously approved these changes May 1, 2024
View reviewed changes
@ijjk
Copy link
Member

ijjk commented May 1, 2024

Failing test suites

Commit: 40ce1bf

pnpm test-start test/e2e/app-dir/app-external/app-external.test.ts

  • app dir - external dependency > should be able to opt-out 3rd party packages being bundled in server components
  • app dir - external dependency > should handle external async module libraries correctly
  • app dir - external dependency > should transpile specific external packages with the transpilePackages option
  • app dir - external dependency > should resolve the subset react in server components based on the react-server condition
  • app dir - external dependency > should resolve 3rd party package exports based on the react-server condition
  • app dir - external dependency > should correctly collect global css imports and mark them as side effects
  • app dir - external dependency > should handle external css modules
  • app dir - external dependency > should use the same export type for packages in both ssr and client
  • app dir - external dependency > should handle external css modules in pages
  • app dir - external dependency > should handle external next/font
  • app dir - external dependency > should not apply swc optimizer transform for external packages in browser layer in web worker
  • app dir - external dependency > should emit cjs helpers for external cjs modules when compiled
  • app dir - external dependency > should export client module references in esm
  • app dir - external dependency > should support exporting multiple star re-exports
  • app dir - external dependency > should have proper tree-shaking for known modules in CJS
  • app dir - external dependency > should use the same async storages if imported directly
  • app dir - external dependency > app route > should resolve next/server api from external esm package
  • app dir - external dependency > mixed syntax external modules > should handle mixed module with next/dynamic
  • app dir - external dependency > mixed syntax external modules > should handle mixed module in server and client components
  • app dir - external dependency > react in external esm packages > should use the same react in client app
  • app dir - external dependency > react in external esm packages > should use the same react in server app
  • app dir - external dependency > react in external esm packages > should use the same react in edge server app
  • app dir - external dependency > react in external esm packages > should use the same react in pages
  • app dir - external dependency > react in external esm packages > should support namespace import with ESM packages
  • app dir - external dependency > server actions > should not prefer to resolve esm over cjs for bundling optout packages
  • app dir - external dependency > server actions > should compile server actions from node_modules in client components
Expand output

● app dir - external dependency › should be able to opt-out 3rd party packages being bundled in server components

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should handle external async module libraries correctly

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should transpile specific external packages with the transpilePackages option

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should resolve the subset react in server components based on the react-server condition

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should resolve 3rd party package exports based on the react-server condition

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should correctly collect global css imports and mark them as side effects

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should handle external css modules

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should use the same export type for packages in both ssr and client

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should handle external css modules in pages

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should handle external next/font

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should not apply swc optimizer transform for external packages in browser layer in web worker

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › react in external esm packages › should use the same react in client app

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › react in external esm packages › should use the same react in server app

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › react in external esm packages › should use the same react in edge server app

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › react in external esm packages › should use the same react in pages

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › react in external esm packages › should support namespace import with ESM packages

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › mixed syntax external modules › should handle mixed module with next/dynamic

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › mixed syntax external modules › should handle mixed module in server and client components

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should emit cjs helpers for external cjs modules when compiled

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should export client module references in esm

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should support exporting multiple star re-exports

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should have proper tree-shaking for known modules in CJS

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › should use the same async storages if imported directly

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › server actions › should not prefer to resolve esm over cjs for bundling optout packages

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › server actions › should compile server actions from node_modules in client components

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

● app dir - external dependency › app route › should resolve next/server api from external esm package

next build failed with code/signal 1

  89 |           if (code || signal)
  90 |             reject(
> 91 |               new Error(`next build failed with code/signal ${code || signal}`)
     |               ^
  92 |             )
  93 |           else resolve()
  94 |         })

  at ChildProcess.<anonymous> (lib/next-modes/next-start.ts:91:15)

Read more about building and testing Next.js in contributing.md.

@huozhi huozhi dismissed their stale review May 1, 2024 18:59

tests failing

@samcx samcx closed this May 14, 2024
@samcx samcx deleted the revert-64209-externalize-udici branch May 14, 2024 05:26
@samcx samcx mentioned this pull request May 14, 2024
Merged
huozhi added a commit that referenced this pull request May 14, 2024
@samcx @huozhi
Revert "chore: externalize undici for bundling" (#65727)
4d97acb 
## Why?

`undici` should be able to compile and bundle now.

- x-ref: #65243

---------

Co-authored-by: Jiachi Liu <inbox@huozhi.im>
panteliselef pushed a commit to panteliselef/next.js that referenced this pull request May 20, 2024
@samcx @huozhi @panteliselef
Revert "chore: externalize undici for bundling" (vercel#65727)
9a5e048 
## Why?

`undici` should be able to compile and bundle now.

- x-ref: vercel#65243

---------

Co-authored-by: Jiachi Liu <inbox@huozhi.im>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@huozhi huozhi huozhi left review comments

@timneutkens timneutkens Awaiting requested review from timneutkens timneutkens is a code owner

@ijjk ijjk Awaiting requested review from ijjk ijjk is a code owner

@shuding shuding Awaiting requested review from shuding shuding is a code owner

@feedthejim feedthejim Awaiting requested review from feedthejim feedthejim is a code owner

@ztanner ztanner Awaiting requested review from ztanner ztanner is a code owner

@wyattjoh wyattjoh Awaiting requested review from wyattjoh wyattjoh is a code owner

@ismaelrumzan ismaelrumzan Awaiting requested review from ismaelrumzan ismaelrumzan is a code owner automatically assigned from vercel/devex

@leerob leerob Awaiting requested review from leerob leerob is a code owner automatically assigned from vercel/devex

Assignees

@samcx samcx

Labels
created-by: Next.js team PRs by the Next.js team Documentation Related to Next.js' official documentation. tests type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@samcx @ijjk @huozhi