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.

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

fix(next-core): apply correct jsx transform context for ssr #57300

Merged
merged 5 commits into from
Oct 24, 2023

Conversation

kwonoj
Copy link
Contributor

@kwonoj kwonoj commented Oct 23, 2023

What

Running test/e2e/app-dir/emotion-js/index.test.ts fails with turbopack as emotion skips necessary runtime transforms for the styles. Digging further, it was due to not applying correct importSource for the transform (@emotion/react) against ssr component when running with turbopack. Peeking next-dev, we apply client side context if issuer layer origin is either ssr or pages for the browser while turbopack doesn't.

PR uses type for the module context as equivalent to the issuer layer, and then apply client context for the transform for those modules.

test/e2e/app-dir/emotion-js/index.test.ts is passing with this PR.

Closes WEB-1834

@ijjk ijjk added Turbopack Related to Turbopack with Next.js. created-by: Turbopack team PRs by the turbopack team type: next labels Oct 23, 2023
@ijjk
Copy link
Member

ijjk commented Oct 24, 2023

Stats from current PR

Default Build
General
vercel/next.js canary vercel/next.js fix-server-jsx-context Change
buildDuration 10.1s 10.1s N/A
buildDurationCached 5.9s 5.8s N/A
nodeModulesSize 175 MB 175 MB
nextStartRea..uration (ms) 420ms 417ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js fix-server-jsx-context Change
199-HASH.js gzip 32.3 kB 32.3 kB N/A
3f784ff6-HASH.js gzip 53.2 kB 53.2 kB N/A
99.HASH.js gzip 182 B 182 B
framework-HASH.js gzip 45.5 kB 45.5 kB
main-app-HASH.js gzip 254 B 252 B N/A
main-HASH.js gzip 35.3 kB 35.4 kB N/A
webpack-HASH.js gzip 1.75 kB 1.75 kB N/A
Overall change 45.7 kB 45.7 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js fix-server-jsx-context Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary vercel/next.js fix-server-jsx-context Change
_app-HASH.js gzip 206 B 205 B N/A
_error-HASH.js gzip 182 B 180 B N/A
amp-HASH.js gzip 506 B 505 B N/A
css-HASH.js gzip 322 B 323 B N/A
dynamic-HASH.js gzip 2.59 kB 2.59 kB
edge-ssr-HASH.js gzip 260 B 259 B N/A
head-HASH.js gzip 350 B 350 B
hooks-HASH.js gzip 369 B 369 B
image-HASH.js gzip 4.38 kB 4.38 kB N/A
index-HASH.js gzip 256 B 256 B
link-HASH.js gzip 2.67 kB 2.67 kB N/A
routerDirect..HASH.js gzip 316 B 318 B N/A
script-HASH.js gzip 385 B 384 B N/A
withRouter-HASH.js gzip 319 B 319 B
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 3.99 kB 3.99 kB
Client Build Manifests
vercel/next.js canary vercel/next.js fix-server-jsx-context Change
_buildManifest.js gzip 484 B 482 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js fix-server-jsx-context Change
index.html gzip 528 B 531 B N/A
link.html gzip 542 B 542 B
withRouter.html gzip 524 B 524 B
Overall change 1.07 kB 1.07 kB
Edge SSR bundle Size
vercel/next.js canary vercel/next.js fix-server-jsx-context Change
edge-ssr.js gzip 96 kB 96 kB N/A
page.js gzip 139 kB 139 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js fix-server-jsx-context Change
middleware-b..fest.js gzip 623 B 625 B N/A
middleware-r..fest.js gzip 150 B 151 B N/A
middleware.js gzip 25.8 kB 25.8 kB
edge-runtime..pack.js gzip 1.92 kB 1.92 kB
Overall change 27.8 kB 27.8 kB
Commit: cc9e34a

@kwonoj kwonoj marked this pull request as ready for review October 24, 2023 00:30
@kwonoj kwonoj changed the title fix(next-core): apply correct jsx transform context for rsc fix(next-core): apply correct jsx transform context for ssr Oct 24, 2023
Copy link
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

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

This looks correct to me.

// This enables correct emotion transform and other hydration between server and
// client bundles. ref: https://github.com/vercel/next.js/blob/4bbf9b6c70d2aa4237defe2bebfa790cdb7e334e/packages/next/src/build/webpack-config.ts#L1421-L1426
let jsx_runtime_options =
get_jsx_transform_options(project_path, mode, None, false, next_config);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be hoisted to the other call site?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can, but currently this is the only place needs this transform options while default transform options are hoisted. If there are other cases need this probably would do it.

@ijjk
Copy link
Member

ijjk commented Oct 24, 2023

Tests Passed

@kodiakhq kodiakhq bot merged commit 8b083c3 into canary Oct 24, 2023
53 of 58 checks passed
@kodiakhq kodiakhq bot deleted the fix-server-jsx-context branch October 24, 2023 21:38
kodiakhq bot pushed a commit that referenced this pull request Oct 24, 2023
### What

Enabling test from #57300. There may be some other test cases passing by fix, but this is the known direct offending test can be enabled.

Closes WEB-1847
@github-actions github-actions bot added the locked label Nov 8, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
created-by: Turbopack team PRs by the turbopack team locked Turbopack Related to Turbopack with Next.js. type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants