Skip to content

Commit

Permalink
remove unused imports tracking on the server graph
Browse files Browse the repository at this point in the history
  • Loading branch information
shuding committed Sep 21, 2024
1 parent 1a6e487 commit 9e1c387
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 165 deletions.
9 changes: 0 additions & 9 deletions packages/next/src/build/webpack/loaders/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,6 @@ import { RSC_MODULE_TYPES } from '../../../shared/lib/constants'
const imageExtensions = ['jpg', 'jpeg', 'png', 'webp', 'avif', 'ico', 'svg']
const imageRegex = new RegExp(`\\.(${imageExtensions.join('|')})$`)

// Determine if the whole module is server action, 'use server' in the top level of module
export function isActionServerLayerEntryModule(mod: {
resource: string
buildInfo?: any
}) {
const rscInfo = mod.buildInfo.rsc
return !!(rscInfo?.actions && rscInfo?.type === RSC_MODULE_TYPES.server)
}

// Determine if the whole module is client action, 'use server' in nested closure in the client module
function isActionClientLayerModule(mod: { resource: string; buildInfo?: any }) {
const rscInfo = mod.buildInfo.rsc
Expand Down
160 changes: 4 additions & 156 deletions packages/next/src/build/webpack/plugins/flight-client-entry-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import {
isClientComponentEntryModule,
isCSSMod,
regexCSS,
isActionServerLayerEntryModule,
} from '../loaders/utils'
import {
traverseModules,
Expand Down Expand Up @@ -76,11 +75,6 @@ const pluginState = getProxiedPluginState({
serverActions: {} as ActionManifest['node'],
edgeServerActions: {} as ActionManifest['edge'],

usedActions: {
node: {} as Record<string, Set<string>>,
edge: {} as Record<string, Set<string>>,
},

actionModServerId: {} as Record<
string,
{
Expand Down Expand Up @@ -163,15 +157,6 @@ function deduplicateCSSImportsForEntry(mergedCSSimports: CssImports) {
return dedupedCSSImports
}

// Collection of action module path and action names per runtime.
type UsedActionMap = {
node: Record<string, Set<string>>
edge: Record<string, Set<string>>
}
type UsedActionPerEntry = {
[entryName: string]: UsedActionMap
}

export class FlightClientEntryPlugin {
dev: boolean
appDir: string
Expand All @@ -180,9 +165,6 @@ export class FlightClientEntryPlugin {
assetPrefix: string
webpackRuntime: string

// Collect the used actions based on the entry name and runtime.
usedActions: UsedActionPerEntry

constructor(options: Options) {
this.dev = options.dev
this.appDir = options.appDir
Expand All @@ -192,40 +174,6 @@ export class FlightClientEntryPlugin {
this.webpackRuntime = this.isEdgeServer
? EDGE_RUNTIME_WEBPACK
: DEFAULT_RUNTIME_WEBPACK

this.usedActions = {}
}

getUsedActionsInEntry(
entryName: string,
modResource: string
): Set<string> | undefined {
const runtime = this.isEdgeServer ? 'edge' : 'node'
const actionsRuntimeMap = this.usedActions[entryName]
const actionMap = actionsRuntimeMap ? actionsRuntimeMap[runtime] : undefined
return actionMap ? actionMap[modResource] : undefined
}

setUsedActionsInEntry(
entryName: string,
modResource: string,
actionNames: string[]
) {
const runtime = this.isEdgeServer ? 'edge' : 'node'
if (!this.usedActions[entryName]) {
this.usedActions[entryName] = {
node: {},
edge: {},
}
}
if (!this.usedActions[entryName][runtime]) {
this.usedActions[entryName][runtime] = {}
}
const actionsMap = this.usedActions[entryName][runtime]
if (!actionsMap[modResource]) {
actionsMap[modResource] = new Set()
}
actionNames.forEach((name) => actionsMap[modResource].add(name))
}

apply(compiler: webpack.Compiler) {
Expand Down Expand Up @@ -339,7 +287,6 @@ export class FlightClientEntryPlugin {

const { clientComponentImports, actionImports, cssImports } =
this.collectComponentInfoFromServerEntryDependency({
entryName: name,
entryRequest,
compilation,
resolvedModule: connection.resolvedModule,
Expand Down Expand Up @@ -502,7 +449,6 @@ export class FlightClientEntryPlugin {
// Collect from all entries, e.g. layout.js, page.js, loading.js, ...
// add aggregate them.
const actionEntryImports = this.collectClientActionsFromDependencies({
entryName: name,
compilation,
dependencies: ssrEntryDependencies,
})
Expand Down Expand Up @@ -560,11 +506,9 @@ export class FlightClientEntryPlugin {
}

collectClientActionsFromDependencies({
entryName,
compilation,
dependencies,
}: {
entryName: string
compilation: webpack.Compilation
dependencies: ReturnType<typeof webpack.EntryPlugin.createDependency>[]
}) {
Expand All @@ -582,45 +526,26 @@ export class FlightClientEntryPlugin {
entryRequest: string
resolvedModule: any
}) => {
const collectActionsInDep = (
mod: webpack.NormalModule,
ids: string[]
): void => {
const collectActionsInDep = (mod: webpack.NormalModule): void => {
if (!mod) return

const modResource = getModuleResource(mod)

if (!modResource) return

const actions = getActionsFromBuildInfo(mod)

// Collect used exported actions.
if (visitedModule.has(modResource) && actions) {
this.setUsedActionsInEntry(entryName, modResource, ids)
}

if (visitedModule.has(modResource)) return

visitedModule.add(modResource)

const actions = getActionsFromBuildInfo(mod)
if (actions) {
collectedActions.set(modResource, actions)
}

// Collect used exported actions transversely.
getModuleReferencesInOrder(mod, compilation.moduleGraph).forEach(
(connection: any) => {
let dependencyIds: string[] = []
const depModule = connection.dependency
if (depModule?.ids) {
dependencyIds.push(...depModule.ids)
} else {
dependencyIds = depModule.category === 'esm' ? [] : ['*']
}

collectActionsInDep(
connection.resolvedModule as webpack.NormalModule,
dependencyIds
connection.resolvedModule as webpack.NormalModule
)
}
)
Expand All @@ -632,7 +557,7 @@ export class FlightClientEntryPlugin {
!entryRequest.includes('next-flight-action-entry-loader')
) {
// Traverse the module graph to find all client components.
collectActionsInDep(resolvedModule, [])
collectActionsInDep(resolvedModule)
}
}

Expand Down Expand Up @@ -662,12 +587,10 @@ export class FlightClientEntryPlugin {
}

collectComponentInfoFromServerEntryDependency({
entryName,
entryRequest,
compilation,
resolvedModule,
}: {
entryName: string
entryRequest: string
compilation: webpack.Compilation
resolvedModule: any /* Dependency */
Expand All @@ -678,7 +601,6 @@ export class FlightClientEntryPlugin {
} {
// Keep track of checked modules to avoid infinite loops with recursive imports.
const visitedOfClientComponentsTraverse = new Set()
const visitedOfActionTraverse = new Set()

// Info to collect.
const clientComponentImports: ClientComponentImports = {}
Expand Down Expand Up @@ -758,56 +680,9 @@ export class FlightClientEntryPlugin {
)
}

const filterUsedActions = (
mod: webpack.NormalModule,
importedIdentifiers: string[]
): void => {
if (!mod) return

const modResource = getModuleResource(mod)

if (!modResource) return
if (visitedOfActionTraverse.has(modResource)) {
if (this.getUsedActionsInEntry(entryName, modResource)) {
this.setUsedActionsInEntry(
entryName,
modResource,
importedIdentifiers
)
}
return
}
visitedOfActionTraverse.add(modResource)

if (isActionServerLayerEntryModule(mod)) {
// `ids` are the identifiers that are imported from the dependency,
// if it's present, it's an array of strings.
this.setUsedActionsInEntry(entryName, modResource, importedIdentifiers)

return
}

getModuleReferencesInOrder(mod, compilation.moduleGraph).forEach(
(connection: any) => {
let dependencyIds: string[] = []
const depModule = connection.dependency
if (depModule?.ids) {
dependencyIds.push(...depModule.ids)
} else {
dependencyIds = depModule.category === 'esm' ? [] : ['*']
}

filterUsedActions(connection.resolvedModule, dependencyIds)
}
)
}

// Traverse the module graph to find all client components.
filterClientComponents(resolvedModule, [])

// Traverse the module graph to find all used actions.
filterUsedActions(resolvedModule, [])

return {
clientComponentImports,
cssImports: CSSImports.size
Expand Down Expand Up @@ -954,29 +829,7 @@ export class FlightClientEntryPlugin {
createdActions: Set<string>
fromClient?: boolean
}) {
// Filter out the unused actions before create action entry.
for (const [filePath, names] of actions.entries()) {
const usedActionNames = this.getUsedActionsInEntry(entryName, filePath)
if (!usedActionNames) continue
const containsAll = usedActionNames.has('*')
if (usedActionNames && !containsAll) {
const filteredNames = names.filter(
(name) => usedActionNames.has(name) || isInlineActionIdentifier(name)
)
actions.set(filePath, filteredNames)
} else if (!containsAll) {
// If we didn't collect the used, we erase them from the collected actions
// to avoid creating the action entry.
if (
names.filter((name) => !isInlineActionIdentifier(name)).length === 0
) {
actions.delete(filePath)
}
}
}

const actionsArray = Array.from(actions.entries())

for (const [dep, actionNames] of actions) {
for (const actionName of actionNames) {
createdActions.add(entryName + '@' + dep + '@' + actionName)
Expand Down Expand Up @@ -1210,8 +1063,3 @@ function getModuleResource(mod: webpack.NormalModule): string {
}
return modResource
}

// x-ref crates/next-custom-transforms/src/transforms/server_actions.rs `gen_ident` funcition
function isInlineActionIdentifier(name: string) {
return name.startsWith('$$ACTION_')
}
20 changes: 20 additions & 0 deletions test/e2e/app-dir/actions/app-action.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1702,4 +1702,24 @@ describe('app-dir action handling', () => {
}, 'success')
})
})

it('should keep actions that are behind export * and nested cases bundled', async () => {
const browser = await next.browser('/reexport', {
pushErrorAsConsoleLog: true,
})

await browser.elementByCss('#test-1').click()

const logs = await browser.log()
expect(
logs.some((log) => log.message.includes('action: test-1'))
).toBeTruthy()

await browser.elementByCss('#test-2').click()

const logs2 = await browser.log()
expect(
logs2.some((log) => log.message.includes('action: test-2'))
).toBeTruthy()
})
})
5 changes: 5 additions & 0 deletions test/e2e/app-dir/actions/app/reexport/actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use server'

export async function action() {
console.log('action: test-1')
}
1 change: 1 addition & 0 deletions test/e2e/app-dir/actions/app/reexport/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './actions'
9 changes: 9 additions & 0 deletions test/e2e/app-dir/actions/app/reexport/nested.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use server'

export async function foo() {
console.log('action: test-2')
}

export async function bar() {
return foo
}
25 changes: 25 additions & 0 deletions test/e2e/app-dir/actions/app/reexport/page.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use client'

import { action } from './index'
import { bar } from './nested'

export default function Page() {
return (
<>
<form action={action}>
<button type="submit" id="test-1">
Submit
</button>
</form>
<button
onClick={async () => {
const foo = await bar()
await foo()
}}
id="test-2"
>
Submit
</button>
</>
)
}

0 comments on commit 9e1c387

Please sign in to comment.