From 51b3f3cb024a13725303ac6f5e36d215ed0c677b Mon Sep 17 00:00:00 2001 From: SeanCassiere <33615041+SeanCassiere@users.noreply.github.com> Date: Sat, 14 Sep 2024 10:35:19 +1200 Subject: [PATCH 01/10] fix(router-plugin): do not split component if ident is being exported --- .../src/core/code-splitter/compilers.ts | 81 +++++++++++-------- 1 file changed, 47 insertions(+), 34 deletions(-) diff --git a/packages/router-plugin/src/core/code-splitter/compilers.ts b/packages/router-plugin/src/core/code-splitter/compilers.ts index b7feabd10c..458813c578 100644 --- a/packages/router-plugin/src/core/code-splitter/compilers.ts +++ b/packages/router-plugin/src/core/code-splitter/compilers.ts @@ -98,6 +98,8 @@ export function compileCodeSplitReferenceRoute(opts: ParseAstOptions) { if (prop.key.name === 'component') { const value = prop.value + let shouldSplit = true + if (t.isIdentifier(value)) { existingCompImportPath = getImportSpecifierAndPathFromLocalName( @@ -105,48 +107,59 @@ export function compileCodeSplitReferenceRoute(opts: ParseAstOptions) { value.name, ).path - removeIdentifierLiteral(path, value) - } + const binding = path.scope.getBinding(value.name) + const referenceCount = ( + binding?.referencePaths || [] + ).length - // Prepend the import statement to the program along with the importer function - // Check to see if lazyRouteComponent is already imported before attempting - // to import it again + shouldSplit = referenceCount <= 1 - if ( - !hasImportedOrDefinedIdentifier( - 'lazyRouteComponent', - ) - ) { - programPath.unshiftContainer('body', [ - template.statement( - `import { lazyRouteComponent } from '@tanstack/react-router'`, - )(), - ]) + if (shouldSplit) { + removeIdentifierLiteral(path, value) + } } - if ( - !hasImportedOrDefinedIdentifier( - '$$splitComponentImporter', - ) - ) { - programPath.unshiftContainer('body', [ + if (shouldSplit) { + // Prepend the import statement to the program along with the importer function + // Check to see if lazyRouteComponent is already imported before attempting + // to import it again + + if ( + !hasImportedOrDefinedIdentifier( + 'lazyRouteComponent', + ) + ) { + programPath.unshiftContainer('body', [ + template.statement( + `import { lazyRouteComponent } from '@tanstack/react-router'`, + )(), + ]) + } + + if ( + !hasImportedOrDefinedIdentifier( + '$$splitComponentImporter', + ) + ) { + programPath.unshiftContainer('body', [ + template.statement( + `const $$splitComponentImporter = () => import('${splitUrl}')`, + )(), + ]) + } + + prop.value = template.expression( + `lazyRouteComponent($$splitComponentImporter, 'component')`, + )() + + programPath.pushContainer('body', [ template.statement( - `const $$splitComponentImporter = () => import('${splitUrl}')`, + `function DummyComponent() { return null }`, )(), ]) - } - prop.value = template.expression( - `lazyRouteComponent($$splitComponentImporter, 'component')`, - )() - - programPath.pushContainer('body', [ - template.statement( - `function DummyComponent() { return null }`, - )(), - ]) - - found = true + found = true + } } else if (prop.key.name === 'loader') { const value = prop.value From 416ad59c999aafefa5594301134c24a566d48495 Mon Sep 17 00:00:00 2001 From: SeanCassiere <33615041+SeanCassiere@users.noreply.github.com> Date: Sat, 14 Sep 2024 11:09:03 +1200 Subject: [PATCH 02/10] fix(router-plugin): better logic for not splitting the component and loader based on its exports --- .../src/core/code-splitter/compilers.ts | 95 +++++++++++++------ 1 file changed, 64 insertions(+), 31 deletions(-) diff --git a/packages/router-plugin/src/core/code-splitter/compilers.ts b/packages/router-plugin/src/core/code-splitter/compilers.ts index 458813c578..04d5f37320 100644 --- a/packages/router-plugin/src/core/code-splitter/compilers.ts +++ b/packages/router-plugin/src/core/code-splitter/compilers.ts @@ -107,12 +107,8 @@ export function compileCodeSplitReferenceRoute(opts: ParseAstOptions) { value.name, ).path - const binding = path.scope.getBinding(value.name) - const referenceCount = ( - binding?.referencePaths || [] - ).length - - shouldSplit = referenceCount <= 1 + const isExported = hasExport(ast, value.name) + shouldSplit = !isExported if (shouldSplit) { removeIdentifierLiteral(path, value) @@ -163,6 +159,8 @@ export function compileCodeSplitReferenceRoute(opts: ParseAstOptions) { } else if (prop.key.name === 'loader') { const value = prop.value + let shouldSplit = true + if (t.isIdentifier(value)) { existingLoaderImportPath = getImportSpecifierAndPathFromLocalName( @@ -170,36 +168,42 @@ export function compileCodeSplitReferenceRoute(opts: ParseAstOptions) { value.name, ).path - removeIdentifierLiteral(path, value) - } - - // Prepend the import statement to the program along with the importer function + const isExported = hasExport(ast, value.name) + shouldSplit = !isExported - if (!hasImportedOrDefinedIdentifier('lazyFn')) { - programPath.unshiftContainer('body', [ - template.smart( - `import { lazyFn } from '@tanstack/react-router'`, - )() as t.Statement, - ]) + if (shouldSplit) { + removeIdentifierLiteral(path, value) + } } - if ( - !hasImportedOrDefinedIdentifier( - '$$splitLoaderImporter', - ) - ) { - programPath.unshiftContainer('body', [ - template.statement( - `const $$splitLoaderImporter = () => import('${splitUrl}')`, - )(), - ]) - } + if (shouldSplit) { + // Prepend the import statement to the program along with the importer function + if (!hasImportedOrDefinedIdentifier('lazyFn')) { + programPath.unshiftContainer('body', [ + template.smart( + `import { lazyFn } from '@tanstack/react-router'`, + )() as t.Statement, + ]) + } + + if ( + !hasImportedOrDefinedIdentifier( + '$$splitLoaderImporter', + ) + ) { + programPath.unshiftContainer('body', [ + template.statement( + `const $$splitLoaderImporter = () => import('${splitUrl}')`, + )(), + ]) + } - prop.value = template.expression( - `lazyFn($$splitLoaderImporter, 'loader')`, - )() + prop.value = template.expression( + `lazyFn($$splitLoaderImporter, 'loader')`, + )() - found = true + found = true + } } } } @@ -530,3 +534,32 @@ function removeIdentifierLiteral(path: any, node: any) { } } } + +function hasExport(ast: t.File, name: string) { + let found = false + babel.traverse(ast, { + ExportNamedDeclaration(path) { + if (path.node.declaration) { + if (t.isVariableDeclaration(path.node.declaration)) { + path.node.declaration.declarations.forEach((decl) => { + if (t.isVariableDeclarator(decl)) { + if (t.isIdentifier(decl.id)) { + if (decl.id.name === name) { + found = true + } + } + } + }) + } + } + }, + ExportDefaultDeclaration(path) { + if (t.isIdentifier(path.node.declaration)) { + if (path.node.declaration.name === name) { + found = true + } + } + }, + }) + return found +} From 205b060b32839b246c564589d07fbe4fb7316c4a Mon Sep 17 00:00:00 2001 From: SeanCassiere <33615041+SeanCassiere@users.noreply.github.com> Date: Sat, 14 Sep 2024 12:08:49 +1200 Subject: [PATCH 03/10] fix(router-plugin): handle the split file case --- .../src/core/code-splitter/compilers.ts | 164 ++++++++++++++---- 1 file changed, 134 insertions(+), 30 deletions(-) diff --git a/packages/router-plugin/src/core/code-splitter/compilers.ts b/packages/router-plugin/src/core/code-splitter/compilers.ts index 04d5f37320..fdbbf02053 100644 --- a/packages/router-plugin/src/core/code-splitter/compilers.ts +++ b/packages/router-plugin/src/core/code-splitter/compilers.ts @@ -107,7 +107,10 @@ export function compileCodeSplitReferenceRoute(opts: ParseAstOptions) { value.name, ).path - const isExported = hasExport(ast, value.name) + // exported identifiers should not be split + // since they are already being imported + // and need to be retained in the compiled file + const isExported = hasExport(ast, value) shouldSplit = !isExported if (shouldSplit) { @@ -168,7 +171,10 @@ export function compileCodeSplitReferenceRoute(opts: ParseAstOptions) { value.name, ).path - const isExported = hasExport(ast, value.name) + // exported identifiers should not be split + // since they are already being imported + // and need to be retained in the compiled file + const isExported = hasExport(ast, value) shouldSplit = !isExported if (shouldSplit) { @@ -269,6 +275,8 @@ export function compileCodeSplitVirtualRoute(opts: ParseAstOptions) { ) } + const knownExportedIdents = new Set() + babel.traverse(ast, { Program: { enter(programPath, programState) { @@ -305,13 +313,28 @@ export function compileCodeSplitVirtualRoute(opts: ParseAstOptions) { if (t.isObjectExpression(options)) { options.properties.forEach((prop) => { if (t.isObjectProperty(prop)) { - splitNodeTypes.forEach((type) => { - if (t.isIdentifier(prop.key)) { - if (prop.key.name === type) { - splitNodesByType[type] = prop.value + if (t.isIdentifier(prop.key)) { + if (splitNodeTypes.includes(prop.key.name as any)) { + const value = prop.value + + let isExported = false + if (t.isIdentifier(value)) { + isExported = hasExport(ast, value) + if (isExported) { + knownExportedIdents.add(value.name) + } + } + + // If the node is exported, we need to remove + // the export from the split file + if (isExported) { + removeExports(ast, value) + } else { + splitNodesByType[prop.key.name as SplitNodeType] = + prop.value } } - }) + } } }) @@ -466,6 +489,28 @@ export function compileCodeSplitVirtualRoute(opts: ParseAstOptions) { deadCodeElimination(ast) + // if there are exported identifiers, then we need to add a warning + // to the file to let the user know that the exported identifiers + // will not in the split file but in the original file, therefore + // increasing the bundle size + if (knownExportedIdents.size > 0) { + const list = Array.from(knownExportedIdents).reduce((str, ident) => { + str += `\n- ${ident}` + return str + }, '') + + const warningMessage = `These exports from "${opts.filename.replace('?' + splitPrefix, '')}" are not being code-split and therefore increasing your bundle size: ${list}\nYou should remove the export statement from these exports or move them to another file that is not a route.` + console.warn(warningMessage) + + // append this warning to the file using a template + if (process.env.NODE_ENV !== 'production') { + const warningTemplate = template.statement( + `console.warn(${JSON.stringify(warningMessage)})`, + )() + ast.program.body.unshift(warningTemplate) + } + } + return generate(ast, { sourceMaps: true, minified: process.env.NODE_ENV === 'production', @@ -535,31 +580,90 @@ function removeIdentifierLiteral(path: any, node: any) { } } -function hasExport(ast: t.File, name: string) { +function hasExport(ast: t.File, node: t.Identifier | unknown) { let found = false - babel.traverse(ast, { - ExportNamedDeclaration(path) { - if (path.node.declaration) { - if (t.isVariableDeclaration(path.node.declaration)) { - path.node.declaration.declarations.forEach((decl) => { - if (t.isVariableDeclarator(decl)) { - if (t.isIdentifier(decl.id)) { - if (decl.id.name === name) { - found = true + + if (!t.isNode(node)) { + throw new Error( + `hasExport() only accepts valid babel nodes, received:` + node, + ) + } + + switch (true) { + case t.isIdentifier(node): + babel.traverse(ast, { + ExportNamedDeclaration(path) { + if (path.node.declaration) { + if (t.isVariableDeclaration(path.node.declaration)) { + path.node.declaration.declarations.forEach((decl) => { + if (t.isVariableDeclarator(decl)) { + if (t.isIdentifier(decl.id)) { + if (decl.id.name === node.name) { + found = true + } + } } - } + }) } - }) - } - } - }, - ExportDefaultDeclaration(path) { - if (t.isIdentifier(path.node.declaration)) { - if (path.node.declaration.name === name) { - found = true - } - } - }, - }) + } + }, + ExportDefaultDeclaration(path) { + if (t.isIdentifier(path.node.declaration)) { + if (path.node.declaration.name === node.name) { + found = true + } + } + }, + }) + break + default: + break + } + return found } + +function removeExports(ast: t.File, node: t.Identifier | unknown) { + let removed = false + + if (!t.isNode(node)) { + throw new Error( + `removeExports() only accepts valid babel nodes, received:` + node, + ) + } + + switch (true) { + case t.isIdentifier(node): + babel.traverse(ast, { + ExportNamedDeclaration(path) { + if (path.node.declaration) { + if (t.isVariableDeclaration(path.node.declaration)) { + path.node.declaration.declarations.forEach((decl) => { + if (t.isVariableDeclarator(decl)) { + if (t.isIdentifier(decl.id)) { + if (decl.id.name === node.name) { + path.remove() + removed = true + } + } + } + }) + } + } + }, + ExportDefaultDeclaration(path) { + if (t.isIdentifier(path.node.declaration)) { + if (path.node.declaration.name === node.name) { + path.remove() + removed = true + } + } + }, + }) + break + default: + break + } + + return removed +} From fbf60c6e52fe912d41299e8ee76696a0301e5707 Mon Sep 17 00:00:00 2001 From: SeanCassiere <33615041+SeanCassiere@users.noreply.github.com> Date: Sat, 14 Sep 2024 12:09:58 +1200 Subject: [PATCH 04/10] test(router-plugin): handling the component and loader being exported as variables --- .../snapshots/retain-exports-const.tsx | 30 ++++++++++++++ .../snapshots/retain-exports-const@split.tsx | 1 + .../test-files/retain-exports-const.tsx | 40 +++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const.tsx create mode 100644 packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx create mode 100644 packages/router-plugin/tests/code-splitter/test-files/retain-exports-const.tsx diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const.tsx new file mode 100644 index 0000000000..69e27260d4 --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const.tsx @@ -0,0 +1,30 @@ +import { createFileRoute, Outlet } from '@tanstack/react-router'; +import { importedComponent as ImportedComponent, importedLoader } from '../shared/imported'; +export const loaderFn = () => { + return importedLoader(); +}; +const Layout = () => { + return
+
+ +
+ + +
; +}; +export const Route = createFileRoute('/_layout')({ + component: Layout, + loader: loaderFn +}); +const HEADER_HEIGHT = '63px'; +export const SIDEBAR_WIDTH = '150px'; +export const SIDEBAR_MINI_WIDTH = '80px'; +export default Layout; \ No newline at end of file diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx new file mode 100644 index 0000000000..0d19512623 --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx @@ -0,0 +1 @@ +console.warn("These exports from \"retain-exports-const.tsx\" are not being code-split and therefore increasing your bundle size: \n- Layout\n- loaderFn\nYou should remove the export statement from these exports or move them to another file that is not a route."); \ No newline at end of file diff --git a/packages/router-plugin/tests/code-splitter/test-files/retain-exports-const.tsx b/packages/router-plugin/tests/code-splitter/test-files/retain-exports-const.tsx new file mode 100644 index 0000000000..c055a3702f --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/test-files/retain-exports-const.tsx @@ -0,0 +1,40 @@ +import * as React from 'react' +import { createFileRoute, Outlet } from '@tanstack/react-router' +import { + importedComponent as ImportedComponent, + importedLoader, +} from '../shared/imported' + +export const loaderFn = () => { + return importedLoader() +} + +const Layout = () => { + return ( +
+
+ +
+ + +
+ ) +} + +export const Route = createFileRoute('/_layout')({ + component: Layout, + loader: loaderFn, +}) + +const HEADER_HEIGHT = '63px' +export const SIDEBAR_WIDTH = '150px' +export const SIDEBAR_MINI_WIDTH = '80px' +const ASIDE_WIDTH = '250px' + +export default Layout From 1c82c288cbe8303dbd1fa87fb53ecbe4fabc53ac Mon Sep 17 00:00:00 2001 From: SeanCassiere <33615041+SeanCassiere@users.noreply.github.com> Date: Sat, 14 Sep 2024 12:47:59 +1200 Subject: [PATCH 05/10] fix(router-plugin): handle named exports that are functions --- .../src/core/code-splitter/compilers.ts | 131 +++++++++--------- 1 file changed, 64 insertions(+), 67 deletions(-) diff --git a/packages/router-plugin/src/core/code-splitter/compilers.ts b/packages/router-plugin/src/core/code-splitter/compilers.ts index fdbbf02053..2bc80dfeec 100644 --- a/packages/router-plugin/src/core/code-splitter/compilers.ts +++ b/packages/router-plugin/src/core/code-splitter/compilers.ts @@ -327,7 +327,7 @@ export function compileCodeSplitVirtualRoute(opts: ParseAstOptions) { // If the node is exported, we need to remove // the export from the split file - if (isExported) { + if (isExported && t.isIdentifier(value)) { removeExports(ast, value) } else { splitNodesByType[prop.key.name as SplitNodeType] = @@ -499,7 +499,7 @@ export function compileCodeSplitVirtualRoute(opts: ParseAstOptions) { return str }, '') - const warningMessage = `These exports from "${opts.filename.replace('?' + splitPrefix, '')}" are not being code-split and therefore increasing your bundle size: ${list}\nYou should remove the export statement from these exports or move them to another file that is not a route.` + const warningMessage = `These exports from "${opts.filename.replace('?' + splitPrefix, '')}" are not being code-split and therefore increasing your bundle size: ${list}\nYou should remove the export statement from these exports or import them from another file that is not a route.` console.warn(warningMessage) // append this warning to the file using a template @@ -580,90 +580,87 @@ function removeIdentifierLiteral(path: any, node: any) { } } -function hasExport(ast: t.File, node: t.Identifier | unknown) { +function hasExport(ast: t.File, node: t.Identifier): boolean { let found = false - if (!t.isNode(node)) { - throw new Error( - `hasExport() only accepts valid babel nodes, received:` + node, - ) - } - - switch (true) { - case t.isIdentifier(node): - babel.traverse(ast, { - ExportNamedDeclaration(path) { - if (path.node.declaration) { - if (t.isVariableDeclaration(path.node.declaration)) { - path.node.declaration.declarations.forEach((decl) => { - if (t.isVariableDeclarator(decl)) { - if (t.isIdentifier(decl.id)) { - if (decl.id.name === node.name) { - found = true - } - } + babel.traverse(ast, { + ExportNamedDeclaration(path) { + if (path.node.declaration) { + // declared as `const loaderFn = () => {}` + if (t.isVariableDeclaration(path.node.declaration)) { + path.node.declaration.declarations.forEach((decl) => { + if (t.isVariableDeclarator(decl)) { + if (t.isIdentifier(decl.id)) { + if (decl.id.name === node.name) { + found = true } - }) + } } - } - }, - ExportDefaultDeclaration(path) { - if (t.isIdentifier(path.node.declaration)) { - if (path.node.declaration.name === node.name) { + }) + } + + // declared as `function loaderFn() {}` + if (t.isFunctionDeclaration(path.node.declaration)) { + if (t.isIdentifier(path.node.declaration.id)) { + if (path.node.declaration.id.name === node.name) { found = true } } - }, - }) - break - default: - break - } + } + } + }, + ExportDefaultDeclaration(path) { + if (t.isIdentifier(path.node.declaration)) { + if (path.node.declaration.name === node.name) { + found = true + } + } + }, + }) return found } -function removeExports(ast: t.File, node: t.Identifier | unknown) { +function removeExports(ast: t.File, node: t.Identifier): boolean { let removed = false - if (!t.isNode(node)) { - throw new Error( - `removeExports() only accepts valid babel nodes, received:` + node, - ) - } - - switch (true) { - case t.isIdentifier(node): - babel.traverse(ast, { - ExportNamedDeclaration(path) { - if (path.node.declaration) { - if (t.isVariableDeclaration(path.node.declaration)) { - path.node.declaration.declarations.forEach((decl) => { - if (t.isVariableDeclarator(decl)) { - if (t.isIdentifier(decl.id)) { - if (decl.id.name === node.name) { - path.remove() - removed = true - } - } + babel.traverse(ast, { + ExportNamedDeclaration(path) { + if (path.node.declaration) { + // declared as `const loaderFn = () => {}` + if (t.isVariableDeclaration(path.node.declaration)) { + path.node.declaration.declarations.forEach((decl) => { + if (t.isVariableDeclarator(decl)) { + if (t.isIdentifier(decl.id)) { + if (decl.id.name === node.name) { + path.remove() + removed = true } - }) + } } - } - }, - ExportDefaultDeclaration(path) { - if (t.isIdentifier(path.node.declaration)) { - if (path.node.declaration.name === node.name) { + }) + } + + // declared as `function loaderFn() {}` + if (t.isFunctionDeclaration(path.node.declaration)) { + if (t.isIdentifier(path.node.declaration.id)) { + if (path.node.declaration.id.name === node.name) { path.remove() removed = true } } - }, - }) - break - default: - break - } + } + } + }, + ExportDefaultDeclaration(path) { + if (t.isIdentifier(path.node.declaration)) { + if (path.node.declaration.name === node.name) { + path.remove() + removed = true + } + } + }, + }) return removed } From 919449e0ea43594b89191e957cdafe1c7aa1f287 Mon Sep 17 00:00:00 2001 From: SeanCassiere <33615041+SeanCassiere@users.noreply.github.com> Date: Sat, 14 Sep 2024 12:49:55 +1200 Subject: [PATCH 06/10] fix(router-plugin): use an else-if --- packages/router-plugin/src/core/code-splitter/compilers.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/router-plugin/src/core/code-splitter/compilers.ts b/packages/router-plugin/src/core/code-splitter/compilers.ts index 2bc80dfeec..4e354192c4 100644 --- a/packages/router-plugin/src/core/code-splitter/compilers.ts +++ b/packages/router-plugin/src/core/code-splitter/compilers.ts @@ -639,10 +639,7 @@ function removeExports(ast: t.File, node: t.Identifier): boolean { } } }) - } - - // declared as `function loaderFn() {}` - if (t.isFunctionDeclaration(path.node.declaration)) { + } else if (t.isFunctionDeclaration(path.node.declaration)) { if (t.isIdentifier(path.node.declaration.id)) { if (path.node.declaration.id.name === node.name) { path.remove() From b59a7b49ec2db1fc79e10848e4f590d0746261a0 Mon Sep 17 00:00:00 2001 From: SeanCassiere <33615041+SeanCassiere@users.noreply.github.com> Date: Sat, 14 Sep 2024 12:51:15 +1200 Subject: [PATCH 07/10] test(router-plugin): check that function exports are also being retained --- .../snapshots/retain-exports-const@split.tsx | 2 +- .../snapshots/retain-exports-function.tsx | 30 ++++++++++++++ .../retain-exports-function@split.tsx | 1 + .../test-files/retain-exports-function.tsx | 40 +++++++++++++++++++ 4 files changed, 72 insertions(+), 1 deletion(-) create mode 100644 packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function.tsx create mode 100644 packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function@split.tsx create mode 100644 packages/router-plugin/tests/code-splitter/test-files/retain-exports-function.tsx diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx index 0d19512623..a999c26e0b 100644 --- a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx @@ -1 +1 @@ -console.warn("These exports from \"retain-exports-const.tsx\" are not being code-split and therefore increasing your bundle size: \n- Layout\n- loaderFn\nYou should remove the export statement from these exports or move them to another file that is not a route."); \ No newline at end of file +console.warn("These exports from \"retain-exports-const.tsx\" are not being code-split and therefore increasing your bundle size: \n- Layout\n- loaderFn\nYou should remove the export statement from these exports or import them from another file that is not a route."); \ No newline at end of file diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function.tsx new file mode 100644 index 0000000000..8162228c27 --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function.tsx @@ -0,0 +1,30 @@ +import { createFileRoute, Outlet } from '@tanstack/react-router'; +import { importedComponent as ImportedComponent, importedLoader } from '../shared/imported'; +export function loaderFn() { + return importedLoader(); +} +function Layout() { + return
+
+ +
+ + +
; +} +export const Route = createFileRoute('/_layout')({ + component: Layout, + loader: loaderFn +}); +const HEADER_HEIGHT = '63px'; +export const SIDEBAR_WIDTH = '150px'; +export const SIDEBAR_MINI_WIDTH = '80px'; +export default Layout; \ No newline at end of file diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function@split.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function@split.tsx new file mode 100644 index 0000000000..9a67042975 --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function@split.tsx @@ -0,0 +1 @@ +console.warn("These exports from \"retain-exports-function.tsx\" are not being code-split and therefore increasing your bundle size: \n- Layout\n- loaderFn\nYou should remove the export statement from these exports or import them from another file that is not a route."); \ No newline at end of file diff --git a/packages/router-plugin/tests/code-splitter/test-files/retain-exports-function.tsx b/packages/router-plugin/tests/code-splitter/test-files/retain-exports-function.tsx new file mode 100644 index 0000000000..f68a6cf422 --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/test-files/retain-exports-function.tsx @@ -0,0 +1,40 @@ +import * as React from 'react' +import { createFileRoute, Outlet } from '@tanstack/react-router' +import { + importedComponent as ImportedComponent, + importedLoader, +} from '../shared/imported' + +export function loaderFn() { + return importedLoader() +} + +function Layout() { + return ( +
+
+ +
+ + +
+ ) +} + +export const Route = createFileRoute('/_layout')({ + component: Layout, + loader: loaderFn, +}) + +const HEADER_HEIGHT = '63px' +export const SIDEBAR_WIDTH = '150px' +export const SIDEBAR_MINI_WIDTH = '80px' +const ASIDE_WIDTH = '250px' + +export default Layout From d890bad354199e5b8d8ae6e8a9df13aabf72aa2f Mon Sep 17 00:00:00 2001 From: SeanCassiere <33615041+SeanCassiere@users.noreply.github.com> Date: Sat, 14 Sep 2024 12:57:43 +1200 Subject: [PATCH 08/10] refactor(router-plugin): simplified code-path for handling the properties inside the virtual split --- .../src/core/code-splitter/compilers.ts | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/packages/router-plugin/src/core/code-splitter/compilers.ts b/packages/router-plugin/src/core/code-splitter/compilers.ts index 4e354192c4..ed5baa95cc 100644 --- a/packages/router-plugin/src/core/code-splitter/compilers.ts +++ b/packages/router-plugin/src/core/code-splitter/compilers.ts @@ -313,28 +313,32 @@ export function compileCodeSplitVirtualRoute(opts: ParseAstOptions) { if (t.isObjectExpression(options)) { options.properties.forEach((prop) => { if (t.isObjectProperty(prop)) { - if (t.isIdentifier(prop.key)) { - if (splitNodeTypes.includes(prop.key.name as any)) { - const value = prop.value + splitNodeTypes.forEach((splitType) => { + if ( + !t.isIdentifier(prop.key) || + prop.key.name !== splitType + ) { + return + } - let isExported = false - if (t.isIdentifier(value)) { - isExported = hasExport(ast, value) - if (isExported) { - knownExportedIdents.add(value.name) - } - } + const value = prop.value - // If the node is exported, we need to remove - // the export from the split file - if (isExported && t.isIdentifier(value)) { - removeExports(ast, value) - } else { - splitNodesByType[prop.key.name as SplitNodeType] = - prop.value + let isExported = false + if (t.isIdentifier(value)) { + isExported = hasExport(ast, value) + if (isExported) { + knownExportedIdents.add(value.name) } } - } + + // If the node is exported, we need to remove + // the export from the split file + if (isExported && t.isIdentifier(value)) { + removeExports(ast, value) + } else { + splitNodesByType[splitType] = prop.value + } + }) } }) From 7eb0300f2b77a4a5a1fa48035338b42f257d728f Mon Sep 17 00:00:00 2001 From: SeanCassiere <33615041+SeanCassiere@users.noreply.github.com> Date: Sat, 14 Sep 2024 13:00:38 +1200 Subject: [PATCH 09/10] refactor(router-plugin): add a more descriptive warning message --- packages/router-plugin/src/core/code-splitter/compilers.ts | 2 +- .../code-splitter/snapshots/retain-exports-const@split.tsx | 2 +- .../code-splitter/snapshots/retain-exports-function@split.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/router-plugin/src/core/code-splitter/compilers.ts b/packages/router-plugin/src/core/code-splitter/compilers.ts index ed5baa95cc..2ca311e7e6 100644 --- a/packages/router-plugin/src/core/code-splitter/compilers.ts +++ b/packages/router-plugin/src/core/code-splitter/compilers.ts @@ -503,7 +503,7 @@ export function compileCodeSplitVirtualRoute(opts: ParseAstOptions) { return str }, '') - const warningMessage = `These exports from "${opts.filename.replace('?' + splitPrefix, '')}" are not being code-split and therefore increasing your bundle size: ${list}\nYou should remove the export statement from these exports or import them from another file that is not a route.` + const warningMessage = `These exports from "${opts.filename.replace('?' + splitPrefix, '')}" are not being code-split and will increase your bundle size: ${list}\nThese should either have their export statements removed or be imported from another file that is not a route.` console.warn(warningMessage) // append this warning to the file using a template diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx index a999c26e0b..7d4c85634d 100644 --- a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-const@split.tsx @@ -1 +1 @@ -console.warn("These exports from \"retain-exports-const.tsx\" are not being code-split and therefore increasing your bundle size: \n- Layout\n- loaderFn\nYou should remove the export statement from these exports or import them from another file that is not a route."); \ No newline at end of file +console.warn("These exports from \"retain-exports-const.tsx\" are not being code-split and will increase your bundle size: \n- Layout\n- loaderFn\nThese should either have their export statements removed or be imported from another file that is not a route."); \ No newline at end of file diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function@split.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function@split.tsx index 9a67042975..f54a3bd29e 100644 --- a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function@split.tsx +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-function@split.tsx @@ -1 +1 @@ -console.warn("These exports from \"retain-exports-function.tsx\" are not being code-split and therefore increasing your bundle size: \n- Layout\n- loaderFn\nYou should remove the export statement from these exports or import them from another file that is not a route."); \ No newline at end of file +console.warn("These exports from \"retain-exports-function.tsx\" are not being code-split and will increase your bundle size: \n- Layout\n- loaderFn\nThese should either have their export statements removed or be imported from another file that is not a route."); \ No newline at end of file From a50bb7af29462ca8c992f28c3b8ad9261f9010e1 Mon Sep 17 00:00:00 2001 From: SeanCassiere <33615041+SeanCassiere@users.noreply.github.com> Date: Sat, 14 Sep 2024 14:25:20 +1200 Subject: [PATCH 10/10] test(router-plugin): add more test cases --- .../snapshots/retain-export-component.tsx | 27 ++++++++++++++ .../retain-export-component@split.tsx | 4 +++ .../snapshots/retain-exports-loader.tsx | 14 ++++++++ .../snapshots/retain-exports-loader@split.tsx | 22 ++++++++++++ .../test-files/retain-export-component.tsx | 34 ++++++++++++++++++ .../test-files/retain-exports-loader.tsx | 35 +++++++++++++++++++ 6 files changed, 136 insertions(+) create mode 100644 packages/router-plugin/tests/code-splitter/snapshots/retain-export-component.tsx create mode 100644 packages/router-plugin/tests/code-splitter/snapshots/retain-export-component@split.tsx create mode 100644 packages/router-plugin/tests/code-splitter/snapshots/retain-exports-loader.tsx create mode 100644 packages/router-plugin/tests/code-splitter/snapshots/retain-exports-loader@split.tsx create mode 100644 packages/router-plugin/tests/code-splitter/test-files/retain-export-component.tsx create mode 100644 packages/router-plugin/tests/code-splitter/test-files/retain-exports-loader.tsx diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-export-component.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-export-component.tsx new file mode 100644 index 0000000000..886cad2399 --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-export-component.tsx @@ -0,0 +1,27 @@ +const $$splitLoaderImporter = () => import('tsr-split:retain-export-component.tsx?tsr-split'); +import { lazyFn } from '@tanstack/react-router'; +import { createFileRoute, Outlet } from '@tanstack/react-router'; +import { importedComponent as ImportedComponent } from '../shared/imported'; +export function Layout() { + return
+
+ +
+ + +
; +} +export const Route = createFileRoute('/_layout')({ + component: Layout, + loader: lazyFn($$splitLoaderImporter, 'loader') +}); +const HEADER_HEIGHT = '63px'; +export const SIDEBAR_WIDTH = '150px'; \ No newline at end of file diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-export-component@split.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-export-component@split.tsx new file mode 100644 index 0000000000..b53badb34f --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-export-component@split.tsx @@ -0,0 +1,4 @@ +console.warn("These exports from \"retain-export-component.tsx\" are not being code-split and will increase your bundle size: \n- Layout\nThese should either have their export statements removed or be imported from another file that is not a route."); +import { importedLoader } from '../shared/imported'; +const loader = importedLoader; +export { loader }; \ No newline at end of file diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-loader.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-loader.tsx new file mode 100644 index 0000000000..d514b8fd86 --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-loader.tsx @@ -0,0 +1,14 @@ +const $$splitComponentImporter = () => import('tsr-split:retain-exports-loader.tsx?tsr-split'); +import { lazyRouteComponent } from '@tanstack/react-router'; +import { createFileRoute } from '@tanstack/react-router'; +export function loaderFn() { + return { + foo: 'bar' + }; +} +export const Route = createFileRoute('/_layout')({ + component: lazyRouteComponent($$splitComponentImporter, 'component'), + loader: loaderFn +}); +export const SIDEBAR_WIDTH = '150px'; +export const SIDEBAR_MINI_WIDTH = '80px'; \ No newline at end of file diff --git a/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-loader@split.tsx b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-loader@split.tsx new file mode 100644 index 0000000000..f74980eee1 --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/snapshots/retain-exports-loader@split.tsx @@ -0,0 +1,22 @@ +console.warn("These exports from \"retain-exports-loader.tsx\" are not being code-split and will increase your bundle size: \n- loaderFn\nThese should either have their export statements removed or be imported from another file that is not a route."); +import { Outlet } from '@tanstack/react-router'; +import { importedComponent as ImportedComponent } from '../shared/imported'; +const HEADER_HEIGHT = '63px'; +const component = function Layout() { + return
+
+ +
+ + +
; +}; +export { component }; \ No newline at end of file diff --git a/packages/router-plugin/tests/code-splitter/test-files/retain-export-component.tsx b/packages/router-plugin/tests/code-splitter/test-files/retain-export-component.tsx new file mode 100644 index 0000000000..4ad35a3f5d --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/test-files/retain-export-component.tsx @@ -0,0 +1,34 @@ +import * as React from 'react' +import { createFileRoute, Outlet } from '@tanstack/react-router' +import { + importedComponent as ImportedComponent, + importedLoader, +} from '../shared/imported' + +export function Layout() { + return ( +
+
+ +
+ + +
+ ) +} + +export const Route = createFileRoute('/_layout')({ + component: Layout, + loader: importedLoader, +}) + +const HEADER_HEIGHT = '63px' +export const SIDEBAR_WIDTH = '150px' +const SIDEBAR_MINI_WIDTH = '80px' +const ASIDE_WIDTH = '250px' diff --git a/packages/router-plugin/tests/code-splitter/test-files/retain-exports-loader.tsx b/packages/router-plugin/tests/code-splitter/test-files/retain-exports-loader.tsx new file mode 100644 index 0000000000..d6c88f19f7 --- /dev/null +++ b/packages/router-plugin/tests/code-splitter/test-files/retain-exports-loader.tsx @@ -0,0 +1,35 @@ +import * as React from 'react' +import { createFileRoute, Outlet } from '@tanstack/react-router' +import { importedComponent as ImportedComponent } from '../shared/imported' + +export function loaderFn() { + return { foo: 'bar' } +} + +function Layout() { + return ( +
+
+ +
+ + +
+ ) +} + +export const Route = createFileRoute('/_layout')({ + component: Layout, + loader: loaderFn, +}) + +const HEADER_HEIGHT = '63px' +export const SIDEBAR_WIDTH = '150px' +export const SIDEBAR_MINI_WIDTH = '80px' +const ASIDE_WIDTH = '250px'