From c7f2c635cb1e9c2e9373836640bae4ce76d514d1 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Tue, 17 May 2022 18:41:17 +0800 Subject: [PATCH 1/3] Fix SWC dynamic transform with suspense but without ssr (#36825) The Babel plugin works fine, so it seems not a runtime issue. fixes https://github.com/vercel/next.js/issues/36636 --- .../next-swc/crates/core/src/next_dynamic.rs | 21 +++++-- .../next-dynamic/with-options/input.js | 5 ++ .../next-dynamic/with-options/output-dev.js | 10 ++++ .../next-dynamic/with-options/output-prod.js | 10 ++++ .../with-options/output-server.js | 10 ++++ test/e2e/dynamic-with-suspense/index.test.ts | 55 +++++++++++++++++++ 6 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 test/e2e/dynamic-with-suspense/index.test.ts diff --git a/packages/next-swc/crates/core/src/next_dynamic.rs b/packages/next-swc/crates/core/src/next_dynamic.rs index fef78a24f828d..b8c2f7fdd120e 100644 --- a/packages/next-swc/crates/core/src/next_dynamic.rs +++ b/packages/next-swc/crates/core/src/next_dynamic.rs @@ -223,6 +223,7 @@ impl Fold for NextDynamicPatcher { })))]; let mut has_ssr_false = false; + let mut has_suspense = false; if expr.args.len() == 2 { if let Expr::Object(ObjectLit { @@ -250,21 +251,29 @@ impl Fold for NextDynamicPatcher { if let Some(Lit::Bool(Bool { value: false, span: _, - })) = match &**value { - Expr::Lit(lit) => Some(lit), - _ => None, - } { + })) = value.as_lit() + { has_ssr_false = true } } + if sym == "suspense" { + if let Some(Lit::Bool(Bool { + value: true, + span: _, + })) = value.as_lit() + { + has_suspense = true + } + } } } } props.extend(options_props.iter().cloned()); } } - - if has_ssr_false && self.is_server { + // Don't need to strip the `loader` argument if suspense is true + // See https://github.com/vercel/next.js/issues/36636 for background + if has_ssr_false && !has_suspense && self.is_server { expr.args[0] = Lit::Null(Null { span: DUMMY_SP }).as_arg(); } diff --git a/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/input.js b/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/input.js index 82cb14f53566d..c258d8fade564 100644 --- a/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/input.js +++ b/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/input.js @@ -9,3 +9,8 @@ const DynamicClientOnlyComponent = dynamic( () => import('../components/hello'), { ssr: false } ) + +const DynamicClientOnlyComponentWithSuspense = dynamic( + () => import('../components/hello'), + { ssr: false, suspense: true } +) diff --git a/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-dev.js b/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-dev.js index b0aba0caa23a0..d4d144e1e364c 100644 --- a/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-dev.js +++ b/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-dev.js @@ -17,3 +17,13 @@ const DynamicClientOnlyComponent = dynamic(()=>import('../components/hello') }, ssr: false }); +const DynamicClientOnlyComponentWithSuspense = dynamic(()=>import('../components/hello') +, { + loadableGenerated: { + modules: [ + "some-file.js -> " + "../components/hello" + ] + }, + ssr: false, + suspense: true +}); diff --git a/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-prod.js b/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-prod.js index 6ba8d24a9b45c..d7e8a3ae55f24 100644 --- a/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-prod.js +++ b/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-prod.js @@ -17,3 +17,13 @@ const DynamicClientOnlyComponent = dynamic(()=>import('../components/hello') }, ssr: false }); +const DynamicClientOnlyComponentWithSuspense = dynamic(()=>import('../components/hello') +, { + loadableGenerated: { + webpack: ()=>[ + require.resolveWeak("../components/hello") + ] + }, + ssr: false, + suspense: true +}); diff --git a/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-server.js b/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-server.js index 453c19d3af245..fbaab8c10c037 100644 --- a/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-server.js +++ b/packages/next-swc/crates/core/tests/fixture/next-dynamic/with-options/output-server.js @@ -16,3 +16,13 @@ const DynamicClientOnlyComponent = dynamic(null, { }, ssr: false }); +const DynamicClientOnlyComponentWithSuspense = dynamic(()=>import('../components/hello') +, { + loadableGenerated: { + modules: [ + "some-file.js -> " + "../components/hello" + ] + }, + ssr: false, + suspense: true +}); diff --git a/test/e2e/dynamic-with-suspense/index.test.ts b/test/e2e/dynamic-with-suspense/index.test.ts new file mode 100644 index 0000000000000..b0f24fb96a1e6 --- /dev/null +++ b/test/e2e/dynamic-with-suspense/index.test.ts @@ -0,0 +1,55 @@ +import { createNext } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { hasRedbox, renderViaHTTP } from 'next-test-utils' +import webdriver from 'next-webdriver' + +const suite = + process.env.NEXT_TEST_REACT_VERSION === '^17' ? describe.skip : describe + +// Skip the suspense test if react version is 17 +suite('dynamic with suspense', () => { + let next: NextInstance + + beforeAll(async () => { + next = await createNext({ + files: { + 'pages/index.js': ` + import { Suspense } from "react"; + import dynamic from "next/dynamic"; + + const Thing = dynamic(() => import("./thing"), { ssr: false, suspense: true }); + + export default function IndexPage() { + return ( +
+

Next.js Example

+ + + +
+ ); + } + `, + 'pages/thing.js': ` + export default function Thing() { + return "Thing"; + } + `, + }, + dependencies: {}, + }) + }) + afterAll(() => next.destroy()) + + it('should render server-side', async () => { + const html = await renderViaHTTP(next.url, '/') + expect(html).toContain('Next.js Example') + expect(html).toContain('Thing') + }) + + it('should render client-side', async () => { + const browser = await webdriver(next.url, '/') + expect(await hasRedbox(browser)).toBe(false) + await browser.close() + }) +}) From 96034e2d9cf2e15c70a49d591fadf4e7a6aecde2 Mon Sep 17 00:00:00 2001 From: Sukka Date: Tue, 17 May 2022 19:05:31 +0800 Subject: [PATCH 2/3] fix(#36651): disable reactRemoveProperties in jest transform (#36922) ## Bug - [x] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have helpful link attached, see `contributing.md` Fixes #36651. Always disable `reactRemoveProperties` in `next/jest` transformation. --- packages/next/build/swc/options.js | 6 +- .../app/jest.config.js | 20 ++++++ .../app/next.config.js | 5 ++ .../app/pages/index.js | 3 + .../remove-react-properties-jest.test.ts | 62 +++++++++++++++++++ 5 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 test/production/jest/remove-react-properties/app/jest.config.js create mode 100644 test/production/jest/remove-react-properties/app/next.config.js create mode 100644 test/production/jest/remove-react-properties/app/pages/index.js create mode 100644 test/production/jest/remove-react-properties/remove-react-properties-jest.test.ts diff --git a/packages/next/build/swc/options.js b/packages/next/build/swc/options.js index eea0abad70265..08ac0e5334e90 100644 --- a/packages/next/build/swc/options.js +++ b/packages/next/build/swc/options.js @@ -105,7 +105,11 @@ function getBaseSWCOptions({ } : null, removeConsole: nextConfig?.compiler?.removeConsole, - reactRemoveProperties: nextConfig?.compiler?.reactRemoveProperties, + // disable "reactRemoveProperties" when "jest" is true + // otherwise the setting from next.config.js will be used + reactRemoveProperties: jest + ? false + : nextConfig?.compiler?.reactRemoveProperties, modularizeImports: nextConfig?.experimental?.modularizeImports, relay: nextConfig?.compiler?.relay, emotion: getEmotionOptions(nextConfig, development), diff --git a/test/production/jest/remove-react-properties/app/jest.config.js b/test/production/jest/remove-react-properties/app/jest.config.js new file mode 100644 index 0000000000000..23f86dd58d7cf --- /dev/null +++ b/test/production/jest/remove-react-properties/app/jest.config.js @@ -0,0 +1,20 @@ +const nextJest = require('next/jest') + +const createJestConfig = nextJest({ + // Provide the path to your Next.js app to load next.config.js and .env files in your test environment + dir: './', +}) + +// Add any custom config to be passed to Jest +const customJestConfig = { + // if using TypeScript with a baseUrl set to the root directory then you need the below for alias' to work + moduleDirectories: ['node_modules', '/'], + testEnvironment: 'jest-environment-jsdom', + moduleNameMapper: { + // When changing these, also look at the tsconfig! + '^types/(.+)$': '/types/$1', + }, +} + +// createJestConfig is exported this way to ensure that next/jest can load the Next.js config which is async +module.exports = createJestConfig(customJestConfig) diff --git a/test/production/jest/remove-react-properties/app/next.config.js b/test/production/jest/remove-react-properties/app/next.config.js new file mode 100644 index 0000000000000..8cb6a9091e1ac --- /dev/null +++ b/test/production/jest/remove-react-properties/app/next.config.js @@ -0,0 +1,5 @@ +module.exports = { + compiler: { + reactRemoveProperties: true, + }, +} diff --git a/test/production/jest/remove-react-properties/app/pages/index.js b/test/production/jest/remove-react-properties/app/pages/index.js new file mode 100644 index 0000000000000..fe30704f5dee7 --- /dev/null +++ b/test/production/jest/remove-react-properties/app/pages/index.js @@ -0,0 +1,3 @@ +export default function Index() { + return
Hello World
+} diff --git a/test/production/jest/remove-react-properties/remove-react-properties-jest.test.ts b/test/production/jest/remove-react-properties/remove-react-properties-jest.test.ts new file mode 100644 index 0000000000000..576fb43af3921 --- /dev/null +++ b/test/production/jest/remove-react-properties/remove-react-properties-jest.test.ts @@ -0,0 +1,62 @@ +import { createNext, FileRef } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { renderViaHTTP } from 'next-test-utils' +import path from 'path' + +const appDir = path.join(__dirname, 'app') + +describe('next/jest', () => { + let next: NextInstance + + if (process.env.NEXT_TEST_REACT_VERSION === '^17') { + // react testing library is specific to react version + it('should bail on react v17', () => {}) + return + } + + beforeAll(async () => { + next = await createNext({ + files: { + pages: new FileRef(path.join(appDir, 'pages')), + 'tests/index.test.js': ` + import { render as renderFn, waitFor } from '@testing-library/react' + import '@testing-library/jest-dom/extend-expect'; + + import Page from '../pages' + + describe('testid', () => { + it('data-testid should be available in the test', async () => { + const { getByTestId } = renderFn( + + ) + expect(getByTestId('main-text')).toHaveTextContent('Hello World') + }) + }) + + `, + 'jest.config.js': new FileRef(path.join(appDir, 'jest.config.js')), + 'next.config.js': new FileRef(path.join(appDir, 'next.config.js')), + }, + dependencies: { + jest: '27.4.7', + '@testing-library/react': '^13.1.1', + jsdom: '^19.0.0', + '@testing-library/jest-dom': '5.16.4', + }, + packageJson: { + scripts: { + // Runs jest and bails if jest fails + build: 'yarn jest --forceExit tests/index.test.js && yarn next build', + }, + }, + buildCommand: `yarn build`, + }) + }) + afterAll(() => next.destroy()) + + it('data-testid should be removed in production', async () => { + const html = await renderViaHTTP(next.appPort, '/') + + expect(html).not.toContain('data-testid') + }) +}) From 16bcb070d8b32a8126debd576f3a9d4fdb6e67f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donny/=EA=B0=95=EB=8F=99=EC=9C=A4?= Date: Tue, 17 May 2022 23:15:24 +0900 Subject: [PATCH 3/3] Update swc (#36972) This PR updates swc to https://github.com/swc-project/swc/commit/f226c0a3d861ed40ebb0e1f593c25920957a3f5b This PR resolves - `https://github.com/vercel/next.js/discussions/30237#discussioncomment-2739632` image Which is related to `styled-components` --- packages/next-swc/Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next-swc/Cargo.lock b/packages/next-swc/Cargo.lock index fcff10f416043..1c9e749b6cd75 100644 --- a/packages/next-swc/Cargo.lock +++ b/packages/next-swc/Cargo.lock @@ -2252,9 +2252,9 @@ dependencies = [ [[package]] name = "swc_ecma_minifier" -version = "0.114.2" +version = "0.114.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c081b070728a95156fdf4589257c2a8321df182432f5f23b395330a8462f3be8" +checksum = "bd914b26110a782b362e97b2cbf697c18a637293b1cccb5d02dc335dca419609" dependencies = [ "ahash", "arrayvec",