Turbopack: app externals test case improvements #62871

sokra · 2024-03-05T09:06:59Z

What?

rename test case
improve error message and handle edge case for require resolving
fix test case actually testing externals (webpack was silently bundling, Turbopack showed error that helped to find the broken test case)
fix order of resolving plugins to handle shared-runtime before externals

Why?

How?

Closes PACK-2662

ijjk · 2024-03-05T09:14:00Z

Failing test suites

Commit: 6c6cbec

TURBOPACK=1 pnpm test test/integration/create-next-app/package-manager.test.ts (turbopack)

should use npm as the package manager on supplying --use-npm with example
should use Yarn as the package manager on supplying --use-yarn with example
should use pnpm as the package manager on supplying --use-pnpm with example
should use Bun as the package manager on supplying --use-bun with example
should infer npm as the package manager with example
should infer yarn as the package manager with example
should infer pnpm as the package manager with example
should infer Bun as the package manager with example

Expand output

● should use npm as the package manager on supplying --use-npm with example

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/package-manager.test.ts:88:28
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/package-manager.test.ts:72:3)

● should use Yarn as the package manager on supplying --use-yarn with example

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/package-manager.test.ts:166:28
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/package-manager.test.ts:150:3)

● should use pnpm as the package manager on supplying --use-pnpm with example

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/package-manager.test.ts:244:28
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/package-manager.test.ts:228:3)

● should use Bun as the package manager on supplying --use-bun with example

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/package-manager.test.ts:318:28
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/package-manager.test.ts:302:3)

● should infer npm as the package manager with example

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/package-manager.test.ts:389:28
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/package-manager.test.ts:366:3)

● should infer yarn as the package manager with example

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/package-manager.test.ts:472:28
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/package-manager.test.ts:449:3)

● should infer pnpm as the package manager with example

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/package-manager.test.ts:555:28
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/package-manager.test.ts:532:3)

● should infer Bun as the package manager with example

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/package-manager.test.ts:630:28
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/package-manager.test.ts:607:3)

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

TURBOPACK=1 pnpm test test/integration/create-next-app/index.test.ts (turbopack)

create next app > valid example
create next app > should allow example with GitHub URL
create next app > should allow example with GitHub URL and example-path
create next app > should use --example-path over the file path in the GitHub URL

Expand output

● create next app › valid example

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/index.test.ts:137:30
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/index.test.ts:121:5)

● create next app › should allow example with GitHub URL

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/index.test.ts:194:30
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/index.test.ts:177:5)

● create next app › should allow example with GitHub URL and example-path

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/index.test.ts:258:30
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/index.test.ts:239:5)

● create next app › should use --example-path over the file path in the GitHub URL

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  80 |   for (const file of files) {
  81 |     try {
> 82 |       expect(existsSync(resolve(projectRoot, file))).toBe(true)
     |                                                      ^
  83 |     } catch (err) {
  84 |       require('console').error(
  85 |         `missing expected file ${file}`,

  at toBe (integration/create-next-app/lib/utils.ts:82:54)
  at integration/create-next-app/index.test.ts:291:30
  at useTempDir (lib/use-temp-dir.ts:24:5)
  at Object.<anonymous> (integration/create-next-app/index.test.ts:272:5)

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

ijjk · 2024-03-05T09:17:33Z

Stats from current PR

Default Build

General

	vercel/next.js canary	vercel/next.js sokra/app-externals	Change
buildDuration	13.5s	13.7s	⚠️ +189ms
buildDurationCached	7.4s	6.6s	N/A
nodeModulesSize	198 MB	198 MB	✓
nextStartRea..uration (ms)	429ms	438ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js sokra/app-externals	Change
2453-HASH.js gzip	30.4 kB	30.4 kB	N/A
3304.HASH.js gzip	181 B	181 B	✓
3f784ff6-HASH.js gzip	53.7 kB	53.7 kB	N/A
8299-HASH.js gzip	5.04 kB	5.04 kB	N/A
framework-HASH.js gzip	45.2 kB	45.2 kB	✓
main-app-HASH.js gzip	242 B	241 B	N/A
main-HASH.js gzip	32.2 kB	32.2 kB	N/A
webpack-HASH.js gzip	1.68 kB	1.68 kB	N/A
Overall change	45.4 kB	45.4 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js sokra/app-externals	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js sokra/app-externals	Change
_app-HASH.js gzip	196 B	197 B	N/A
_error-HASH.js gzip	184 B	184 B	✓
amp-HASH.js gzip	505 B	505 B	✓
css-HASH.js gzip	324 B	325 B	N/A
dynamic-HASH.js gzip	2.5 kB	2.5 kB	N/A
edge-ssr-HASH.js gzip	258 B	258 B	✓
head-HASH.js gzip	352 B	352 B	✓
hooks-HASH.js gzip	370 B	371 B	N/A
image-HASH.js gzip	4.21 kB	4.21 kB	✓
index-HASH.js gzip	259 B	259 B	✓
link-HASH.js gzip	2.67 kB	2.67 kB	N/A
routerDirect..HASH.js gzip	314 B	312 B	N/A
script-HASH.js gzip	386 B	386 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	6.57 kB	6.57 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js sokra/app-externals	Change
_buildManifest.js gzip	481 B	484 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js sokra/app-externals	Change
index.html gzip	529 B	529 B	✓
link.html gzip	542 B	541 B	N/A
withRouter.html gzip	525 B	523 B	N/A
Overall change	529 B	529 B	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js sokra/app-externals	Change
edge-ssr.js gzip	95.1 kB	95.1 kB	N/A
page.js gzip	3.05 kB	3.05 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	vercel/next.js sokra/app-externals	Change
middleware-b..fest.js gzip	626 B	624 B	N/A
middleware-r..fest.js gzip	151 B	151 B	✓
middleware.js gzip	25.5 kB	25.5 kB	N/A
edge-runtime..pack.js gzip	839 B	839 B	✓
Overall change	990 B	990 B	✓

Next Runtimes

	vercel/next.js canary	vercel/next.js sokra/app-externals	Change
app-page-exp...dev.js gzip	171 kB	171 kB	✓
app-page-exp..prod.js gzip	96.6 kB	96.6 kB	✓
app-page-tur..prod.js gzip	98.3 kB	98.3 kB	✓
app-page-tur..prod.js gzip	92.8 kB	92.8 kB	✓
app-page.run...dev.js gzip	149 kB	149 kB	✓
app-page.run..prod.js gzip	91.3 kB	91.3 kB	✓
app-route-ex...dev.js gzip	21.3 kB	21.3 kB	✓
app-route-ex..prod.js gzip	15 kB	15 kB	✓
app-route-tu..prod.js gzip	15 kB	15 kB	✓
app-route-tu..prod.js gzip	14.8 kB	14.8 kB	✓
app-route.ru...dev.js gzip	21 kB	21 kB	✓
app-route.ru..prod.js gzip	14.8 kB	14.8 kB	✓
pages-api-tu..prod.js gzip	9.52 kB	9.52 kB	✓
pages-api.ru...dev.js gzip	9.8 kB	9.8 kB	✓
pages-api.ru..prod.js gzip	9.52 kB	9.52 kB	✓
pages-turbo...prod.js gzip	22.3 kB	22.3 kB	✓
pages.runtim...dev.js gzip	22.9 kB	22.9 kB	✓
pages.runtim..prod.js gzip	22.3 kB	22.3 kB	✓
server.runti..prod.js gzip	50.6 kB	50.6 kB	✓
Overall change	948 kB	948 kB	✓

build cache

	vercel/next.js canary	vercel/next.js sokra/app-externals	Change
0.pack gzip	1.56 MB	1.56 MB	N/A
index.pack gzip	105 kB	105 kB	N/A
Overall change	0 B	0 B	✓

Diff details

Diff for middleware.js

Diff too large to display

Commit: 8bea9a6

…nals in webpack

sokra · 2024-03-08T08:44:04Z

This NextRouter was not mounted. error is weird... Maybe it's really broken...

timneutkens · 2024-03-08T14:13:33Z

The PR is failing on this test suite: TURBOPACK=1 pnpm test-dev test/e2e/getserversideprops/test/index.test.ts

 page error Error: NextRouter was not mounted. https://nextjs.org/docs/messages/next-router-not-mounted
        at useRouter (file:///private/var/folders/sk/pyr4sjxd7bg5dn69d5f_4bsh0000gn/T/next-install-4df8c5dd887386753de664497700ce53fc00b3c42387ec0038229e40ed9e2494/.next/server/chunks/node_modules__pnpm_f99805._.js:8954:15)
        at RouterComp (file:///private/var/folders/sk/pyr4sjxd7bg5dn69d5f_4bsh0000gn/T/next-install-4df8c5dd887386753de664497700ce53fc00b3c42387ec0038229e40ed9e2494/.next/server/chunks/pages_9225a2._.js:65:276)

ijjk added area: tests Turbopack Related to Turbopack with Next.js. created-by: Turbopack team PRs by the turbopack team type: next labels Mar 5, 2024

sokra force-pushed the sokra/app-externals branch 3 times, most recently from b2b51bf to 970acb2 Compare March 5, 2024 15:51

sokra added 5 commits March 5, 2024 16:08

rename test case

6d02e75

improve error message and handle edge case for require resolving

f6b342a

try to append .js for esm externals

65a3db2

update test case to avoid error in turbopack and correctly test exter…

45f37a2

…nals in webpack

fixup externals

6e12d8b

sokra force-pushed the sokra/app-externals branch from 46dba70 to 6e12d8b Compare March 5, 2024 16:08

sokra added 2 commits March 5, 2024 17:29

avoid automatically making potential invalid packages external

e90fc71

update test case for fixed behavior

ff46ceb

sokra marked this pull request as ready for review March 6, 2024 18:29

sokra requested review from timneutkens, ijjk, shuding, huozhi, a team, feedthejim, ztanner and wyattjoh as code owners March 6, 2024 18:29

sokra added 2 commits March 6, 2024 19:29

Merge branch 'canary' into sokra/app-externals

3b261a6

Merge branch 'canary' into sokra/app-externals

7c70191

timneutkens approved these changes Mar 7, 2024

View reviewed changes

sokra enabled auto-merge (squash) March 7, 2024 16:29

sokra and others added 3 commits March 7, 2024 17:31

Merge branch 'canary' into sokra/app-externals

6c6cbec

Merge branch 'canary' into sokra/app-externals

7651052

Merge branch 'canary' into sokra/app-externals

6388697

timneutkens approved these changes Mar 8, 2024

View reviewed changes

sokra added 3 commits March 13, 2024 08:45

Merge branch 'canary' into sokra/app-externals

b5e3d2f

Merge remote-tracking branch 'origin/canary' into sokra/app-externals

e4119b9

fix order of resolve plugins

8bea9a6

sokra merged commit efc5ae4 into canary Mar 14, 2024
68 of 69 checks passed

sokra deleted the sokra/app-externals branch March 14, 2024 07:22

github-actions bot added the locked label Mar 28, 2024

github-actions bot locked as resolved and limited conversation to collaborators Mar 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turbopack: app externals test case improvements #62871

Turbopack: app externals test case improvements #62871

sokra commented Mar 5, 2024 •

edited

ijjk commented Mar 5, 2024 •

edited

ijjk commented Mar 5, 2024 •

edited

sokra commented Mar 8, 2024

timneutkens commented Mar 8, 2024

Turbopack: app externals test case improvements #62871

Turbopack: app externals test case improvements #62871

Conversation

sokra commented Mar 5, 2024 • edited

What?

Why?

How?

ijjk commented Mar 5, 2024 • edited

Failing test suites

ijjk commented Mar 5, 2024 • edited

Stats from current PR

sokra commented Mar 8, 2024

timneutkens commented Mar 8, 2024

sokra commented Mar 5, 2024 •

edited

ijjk commented Mar 5, 2024 •

edited

ijjk commented Mar 5, 2024 •

edited