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

Remove unused Server IDs tracking on the server graph #70317

Merged
merged 4 commits into from
Sep 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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('$$RSC_SERVER_')
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import {
markLayoutAsEdge,
} from '../_testing/utils'

describe('actions-tree-shaking - basic', () => {
// 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,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import {
markLayoutAsEdge,
} from '../_testing/utils'

describe('actions-tree-shaking - mixed-module-actions', () => {
// 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,
})
Expand Down
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')
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './action-modules'
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use client'

import { action } from '../actions'
import { getFoo } from '../nested'

export default function Page() {
return (
<>
<form action={action}>
<button type="submit" id="test-1">
Test 1 Submit
</button>
</form>
<button
onClick={async () => {
const foo = await getFoo()
await foo()
}}
id="test-2"
>
Test 2 Submit
</button>
</>
)
}
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 getFoo() {
return foo
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { action } from '../actions'
import { getFoo } from '../nested'

export default async function Page() {
const foo = await getFoo()
await action()
await foo()
return <>server</>
}

export const dynamic = 'force-dynamic'
Loading
Loading