From 69cc81a06ac1150fcbd15fcc3b18652500a15630 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 22 Sep 2024 21:51:42 +0200 Subject: [PATCH 1/4] test: update SA tree-shaking test --- .../actions-tree-shaking/basic/basic.test.ts | 49 +++++---- .../mixed-module-actions.test.ts | 41 +++---- .../reexport/reexport.test.ts | 101 +++++++++--------- .../shared-module-actions.test.ts | 51 ++++----- .../use-effect-actions.test.ts | 3 +- test/turbopack-build-tests-manifest.json | 90 ++++++++++++++++ 6 files changed, 218 insertions(+), 117 deletions(-) diff --git a/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts b/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts index 6f1c38a18aef9..810cf20095e81 100644 --- a/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts +++ b/test/production/app-dir/actions-tree-shaking/basic/basic.test.ts @@ -5,30 +5,33 @@ import { } from '../_testing/utils' // TODO: revisit when we have a better side-effect free transform approach for server action -describe.skip('actions-tree-shaking - basic', () => { - const { next } = nextTestSetup({ - files: __dirname, - }) +;(process.env.TURBOPACK ? describe : describe.skip)( + 'actions-tree-shaking - basic', + () => { + const { next } = nextTestSetup({ + files: __dirname, + }) - if (process.env.TEST_EDGE) { - markLayoutAsEdge(next) - } + if (process.env.TEST_EDGE) { + markLayoutAsEdge(next) + } - it('should not have the unused action in the manifest', async () => { - const actionsRoutesState = await getActionsRoutesStateByRuntime(next) + it('should not have the unused action in the manifest', async () => { + const actionsRoutesState = await getActionsRoutesStateByRuntime(next) - expect(actionsRoutesState).toMatchObject({ - // only one server layer action - 'app/server/page': { - rsc: 1, - }, - // only one browser layer action - 'app/client/page': { - 'action-browser': 1, - }, - 'app/inline/page': { - rsc: 1, - }, + expect(actionsRoutesState).toMatchObject({ + // only one server layer action + 'app/server/page': { + rsc: 1, + }, + // only one browser layer action + 'app/client/page': { + 'action-browser': 1, + }, + 'app/inline/page': { + rsc: 1, + }, + }) }) - }) -}) + } +) diff --git a/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts b/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts index 85820ecc8a372..1588b639a4c7e 100644 --- a/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts +++ b/test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts @@ -5,26 +5,29 @@ import { } from '../_testing/utils' // TODO: revisit when we have a better side-effect free transform approach for server action -describe.skip('actions-tree-shaking - mixed-module-actions', () => { - const { next } = nextTestSetup({ - files: __dirname, - }) +;(process.env.TURBOPACK ? describe : describe.skip)( + 'actions-tree-shaking - mixed-module-actions', + () => { + const { next } = nextTestSetup({ + files: __dirname, + }) - if (process.env.TEST_EDGE) { - markLayoutAsEdge(next) - } + if (process.env.TEST_EDGE) { + markLayoutAsEdge(next) + } - it('should not do tree shake for cjs module when import server actions', async () => { - const actionsRoutesState = await getActionsRoutesStateByRuntime(next) + it('should not do tree shake for cjs module when import server actions', async () => { + const actionsRoutesState = await getActionsRoutesStateByRuntime(next) - expect(actionsRoutesState).toMatchObject({ - 'app/mixed-module/esm/page': { - rsc: 1, - }, - // CJS import is not able to tree shake, so it will include all actions - 'app/mixed-module/cjs/page': { - rsc: 3, - }, + expect(actionsRoutesState).toMatchObject({ + 'app/mixed-module/esm/page': { + rsc: 1, + }, + // CJS import is not able to tree shake, so it will include all actions + 'app/mixed-module/cjs/page': { + rsc: 3, + }, + }) }) - }) -}) + } +) diff --git a/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts b/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts index 39083bbbb6264..213caa860b0c7 100644 --- a/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts +++ b/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts @@ -6,63 +6,66 @@ import { import { retry } from 'next-test-utils' // TODO: revisit when we have a better side-effect free transform approach for server action -describe.skip('actions-tree-shaking - reexport', () => { - const { next } = nextTestSetup({ - files: __dirname, - skipDeployment: true, - }) +;(process.env.TURBOPACK ? describe : describe.skip)( + 'actions-tree-shaking - reexport', + () => { + const { next } = nextTestSetup({ + files: __dirname, + skipDeployment: true, + }) - if (process.env.TEST_EDGE) { - markLayoutAsEdge(next) - } + if (process.env.TEST_EDGE) { + markLayoutAsEdge(next) + } - it('should not tree-shake namespace exports the manifest', async () => { - const actionsRoutesState = await getActionsRoutesStateByRuntime(next) + it('should not tree-shake namespace exports the manifest', async () => { + const actionsRoutesState = await getActionsRoutesStateByRuntime(next) - expect(actionsRoutesState).toMatchObject({ - 'app/namespace-reexport/server/page': { - rsc: 3, - }, - 'app/namespace-reexport/client/page': { - 'action-browser': 3, - }, - 'app/named-reexport/server/page': { - rsc: 3, - }, - 'app/namespace-reexport-2/client/page': { - 'action-browser': 3, - }, - 'app/namespace-reexport-2/server/page': { - rsc: 3, - }, + expect(actionsRoutesState).toMatchObject({ + 'app/namespace-reexport/server/page': { + rsc: 3, + }, + 'app/namespace-reexport/client/page': { + 'action-browser': 3, + }, + 'app/named-reexport/server/page': { + rsc: 3, + }, + 'app/namespace-reexport-2/client/page': { + 'action-browser': 3, + }, + 'app/namespace-reexport-2/server/page': { + rsc: 3, + }, + }) }) - }) - it('should keep all the action exports for namespace export case on client layer', async () => { - const browser = await next.browser('/namespace-reexport-2/client') - const outputSize = next.cliOutput.length + it('should keep all the action exports for namespace export case on client layer', async () => { + const browser = await next.browser('/namespace-reexport-2/client') + const outputSize = next.cliOutput.length - await browser.elementByCss('#test-1').click() - await retry(async () => { - const output = next.cliOutput.slice(outputSize) - expect(output).toContain('action: test-1') - }) + await browser.elementByCss('#test-1').click() + await retry(async () => { + const output = next.cliOutput.slice(outputSize) + expect(output).toContain('action: test-1') + }) - await browser.elementByCss('#test-2').click() - await retry(async () => { - const output = next.cliOutput.slice(outputSize) - expect(output).toContain('action: test-2') + await browser.elementByCss('#test-2').click() + await retry(async () => { + const output = next.cliOutput.slice(outputSize) + expect(output).toContain('action: test-2') + }) }) - }) - it('should keep all the action exports for namespace export case on server layer', async () => { - const outputSize = next.cliOutput.length - await next.browser('/namespace-reexport-2/server') + it('should keep all the action exports for namespace export case on server layer', async () => { + const outputSize = next.cliOutput.length + await next.browser('/namespace-reexport-2/server') - await retry(async () => { - const output = next.cliOutput.slice(outputSize) - expect(output).toContain('action: test-1') - expect(output).toContain('action: test-2') + await retry(async () => { + const output = next.cliOutput.slice(outputSize) + expect(output).toContain('action: test-1') + expect(output).toContain('action: test-2') + }) }) - }) -}) + } +) diff --git a/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts b/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts index 993200a99f9c9..eb2b7a9a7ee60 100644 --- a/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts +++ b/test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts @@ -5,31 +5,34 @@ import { } from '../_testing/utils' // TODO: revisit when we have a better side-effect free transform approach for server action -describe.skip('actions-tree-shaking - shared-module-actions', () => { - const { next } = nextTestSetup({ - files: __dirname, - }) +;(process.env.TURBOPACK ? describe : describe.skip)( + 'actions-tree-shaking - shared-module-actions', + () => { + const { next } = nextTestSetup({ + files: __dirname, + }) - if (process.env.TEST_EDGE) { - markLayoutAsEdge(next) - } + if (process.env.TEST_EDGE) { + markLayoutAsEdge(next) + } - it('should not have the unused action in the manifest', async () => { - const actionsRoutesState = await getActionsRoutesStateByRuntime(next) + it('should not have the unused action in the manifest', async () => { + const actionsRoutesState = await getActionsRoutesStateByRuntime(next) - expect(actionsRoutesState).toMatchObject({ - 'app/server/one/page': { - rsc: 1, - }, - 'app/server/two/page': { - rsc: 1, - }, - 'app/client/one/page': { - 'action-browser': 1, - }, - 'app/client/two/page': { - 'action-browser': 1, - }, + expect(actionsRoutesState).toMatchObject({ + 'app/server/one/page': { + rsc: 1, + }, + 'app/server/two/page': { + rsc: 1, + }, + 'app/client/one/page': { + 'action-browser': 1, + }, + 'app/client/two/page': { + 'action-browser': 1, + }, + }) }) - }) -}) + } +) diff --git a/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts b/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts index ce3f7da3f6010..9bf34ef7b669d 100644 --- a/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts +++ b/test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts @@ -4,8 +4,7 @@ import { markLayoutAsEdge, } from '../_testing/utils' -// TODO: revisit when we have a better side-effect free transform approach for server action -describe.skip('actions-tree-shaking - use-effect-actions', () => { +describe('actions-tree-shaking - use-effect-actions', () => { const { next } = nextTestSetup({ files: __dirname, }) diff --git a/test/turbopack-build-tests-manifest.json b/test/turbopack-build-tests-manifest.json index 3e869d3816662..6f981890f1017 100644 --- a/test/turbopack-build-tests-manifest.json +++ b/test/turbopack-build-tests-manifest.json @@ -15597,6 +15597,96 @@ "flakey": [], "runtimeError": false }, + "test/production/app-dir/actions-tree-shaking/basic/basic-edge.test.ts": { + "passed": [], + "failed": [ + "actions-tree-shaking - basic should not have the unused action in the manifest" + ], + "pending": [], + "flakey": [], + "runtimeError": false + }, + "test/production/app-dir/actions-tree-shaking/basic/basic.test.ts": { + "passed": [ + "actions-tree-shaking - basic should not have the unused action in the manifest" + ], + "failed": [], + "pending": [], + "flakey": [], + "runtimeError": false + }, + "test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions-edge.test.ts": { + "passed": [ + "actions-tree-shaking - mixed-module-actions should not do tree shake for cjs module when import server actions" + ], + "failed": [], + "pending": [], + "flakey": [], + "runtimeError": false + }, + "test/production/app-dir/actions-tree-shaking/mixed-module-actions/mixed-module-actions.test.ts": { + "passed": [ + "actions-tree-shaking - mixed-module-actions should not do tree shake for cjs module when import server actions" + ], + "failed": [], + "pending": [], + "flakey": [], + "runtimeError": false + }, + "test/production/app-dir/actions-tree-shaking/reexport/reexport-edge.test.ts": { + "passed": [ + "actions-tree-shaking - reexport should not have the unused action in the manifest" + ], + "failed": [], + "pending": [], + "flakey": [], + "runtimeError": false + }, + "test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts": { + "passed": [ + "actions-tree-shaking - reexport should not have the unused action in the manifest" + ], + "failed": [], + "pending": [], + "flakey": [], + "runtimeError": false + }, + "test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions-edge.test.ts": { + "passed": [ + "actions-tree-shaking - shared-module-actions should not have the unused action in the manifest" + ], + "failed": [], + "pending": [], + "flakey": [], + "runtimeError": false + }, + "test/production/app-dir/actions-tree-shaking/shared-module-actions/shared-module-actions.test.ts": { + "passed": [ + "actions-tree-shaking - shared-module-actions should not have the unused action in the manifest" + ], + "failed": [], + "pending": [], + "flakey": [], + "runtimeError": false + }, + "test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions-edge.test.ts": { + "passed": [], + "failed": [ + "actions-tree-shaking - use-effect-actions should not tree shake the used action under useEffect" + ], + "pending": [], + "flakey": [], + "runtimeError": false + }, + "test/production/app-dir/actions-tree-shaking/use-effect-actions/use-effect-actions.test.ts": { + "passed": [], + "failed": [ + "actions-tree-shaking - use-effect-actions should not tree shake the used action under useEffect" + ], + "pending": [], + "flakey": [], + "runtimeError": false + }, "test/production/app-dir/actions-tree-shaking/client-actions-tree-shaking/client-actions-tree-shaking.test.ts": { "passed": [], "failed": [ From a3948765685ffbfbb7650c41588318a90b370069 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 22 Sep 2024 22:23:27 +0200 Subject: [PATCH 2/4] revert test --- .../reexport/reexport.test.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts b/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts index 213caa860b0c7..b7bbaf4e082dc 100644 --- a/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts +++ b/test/production/app-dir/actions-tree-shaking/reexport/reexport.test.ts @@ -23,20 +23,21 @@ import { retry } from 'next-test-utils' expect(actionsRoutesState).toMatchObject({ 'app/namespace-reexport/server/page': { - rsc: 3, + // Turbopack does not tree-shake server side chunks + rsc: process.env.TURBOPACK ? 3 : 1, }, 'app/namespace-reexport/client/page': { - 'action-browser': 3, + // Turbopack does not support tree-shaking export * as we don't have global information + 'action-browser': process.env.TURBOPACK ? 3 : 1, }, + // We're not able to tree-shake these re-exports here in webpack mode 'app/named-reexport/server/page': { - rsc: 3, + // Turbopack supports tree-shaking these re-exports + rsc: process.env.TURBOPACK ? 1 : 3, }, - 'app/namespace-reexport-2/client/page': { + 'app/named-reexport/client/page': { 'action-browser': 3, }, - 'app/namespace-reexport-2/server/page': { - rsc: 3, - }, }) }) From 6d78fe9976690edd8f127b9361bb8034ece0733c Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 22 Sep 2024 23:35:53 +0200 Subject: [PATCH 3/4] update turbopack manifest --- test/turbopack-build-tests-manifest.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/turbopack-build-tests-manifest.json b/test/turbopack-build-tests-manifest.json index 6f981890f1017..d769c78fb433d 100644 --- a/test/turbopack-build-tests-manifest.json +++ b/test/turbopack-build-tests-manifest.json @@ -15646,7 +15646,9 @@ "passed": [ "actions-tree-shaking - reexport should not have the unused action in the manifest" ], - "failed": [], + "failed": [ + "actions-tree-shaking - reexport should keep all the action exports for namespace export case on client layer" + ], "pending": [], "flakey": [], "runtimeError": false From e5fa1139140480bc081049597ca6bc9d5a731780 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 23 Sep 2024 00:11:17 +0200 Subject: [PATCH 4/4] update snapshot --- test/turbopack-build-tests-manifest.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/turbopack-build-tests-manifest.json b/test/turbopack-build-tests-manifest.json index d769c78fb433d..fa25a9ff61653 100644 --- a/test/turbopack-build-tests-manifest.json +++ b/test/turbopack-build-tests-manifest.json @@ -15637,7 +15637,9 @@ "passed": [ "actions-tree-shaking - reexport should not have the unused action in the manifest" ], - "failed": [], + "failed": [ + "actions-tree-shaking - reexport should keep all the action exports for namespace export case on client layer" + ], "pending": [], "flakey": [], "runtimeError": false