Skip to content

Commit

Permalink
Revert "Fix esm property def in flight loader (#66990)" (#69749)
Browse files Browse the repository at this point in the history
This reverts commit 9bb06c5.

This was a sort breaking change that could effect using `next/dynamic`
in server component, when the imported one is client component. Ideally
you shouldn't directly return it due to the property access restriction
for client reference. We've already provided related codemod for v15 but
this might be a bit surprise for users upgrading to the new patch.

Reverted as it's not necessarily part of ESM externals fixes.

Fixes #69720
  • Loading branch information
huozhi authored Sep 6, 2024
1 parent 63b999c commit a882e6e
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 37 deletions.
13 changes: 3 additions & 10 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -483,13 +483,6 @@ export default async function getBaseWebpackConfig(
babel: useSWCLoader ? swcDefaultLoader : babelLoader!,
}

const nextFlightLoader = {
loader: 'next-flight-loader',
options: {
isEdgeServer,
},
}

const appServerLayerLoaders = hasAppDir
? [
// When using Babel, we will have to add the SWC loader
Expand All @@ -502,7 +495,6 @@ export default async function getBaseWebpackConfig(
: []

const instrumentLayerLoaders = [
nextFlightLoader,
// When using Babel, we will have to add the SWC loader
// as an additional pass to handle RSC correctly.
// This will cause some performance overhead but
Expand All @@ -512,7 +504,6 @@ export default async function getBaseWebpackConfig(
].filter(Boolean)

const middlewareLayerLoaders = [
nextFlightLoader,
// When using Babel, we will have to use SWC to do the optimization
// for middleware to tree shake the unused default optimized imports like "next/server".
// This will cause some performance overhead but
Expand Down Expand Up @@ -1358,7 +1349,9 @@ export default async function getBaseWebpackConfig(
isEdgeServer,
}),
},
use: nextFlightLoader,
use: {
loader: 'next-flight-loader',
},
},
]
: []),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ export default function transformSource(
throw new Error('Expected source to have been transformed to a string.')
}

const options = this.getOptions()
const { isEdgeServer } = options

// Assign the RSC meta information to buildInfo.
// Exclude next internal files which are not marked as client files
const buildInfo = getModuleBuildInfo(this._module)
Expand Down Expand Up @@ -100,31 +97,21 @@ export default function transformSource(
return
}

// `proxy` is the module proxy that we treat the module as a client boundary.
// For ESM, we access the property of the module proxy directly for each export.
// This is bit hacky that treating using a CJS like module proxy for ESM's exports,
// but this will avoid creating nested proxies for each export. It will be improved in the future.

// Explanation for: await createProxy(...)
// We need to await the module proxy creation because it can be async module for SSR layer
// due to having async dependencies.
// We only apply `the await` for Node.js as only Edge doesn't have external dependencies.
let esmSource = `\
import { createProxy } from "${MODULE_PROXY_PATH}"
const proxy = ${
isEdgeServer ? '' : 'await'
} createProxy(String.raw\`${resourceKey}\`)
`
let cnt = 0
for (const ref of clientRefs) {
if (ref === '') {
esmSource += `exports[''] = proxy['']\n`
esmSource += `\nexports[''] = createProxy(String.raw\`${resourceKey}#\`);`
} else if (ref === 'default') {
esmSource += `export default proxy.default;\n`
esmSource += `\
export default createProxy(String.raw\`${resourceKey}#default\`);
`
} else {
esmSource += `const e${cnt} = proxy["${ref}"];\n`
esmSource += `export { e${cnt++} as ${ref} };\n`
esmSource += `
const e${cnt} = createProxy(String.raw\`${resourceKey}#${ref}\`);
export { e${cnt++} as ${ref} };`
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ async function createComponentTreeInternal({
}

const LayoutOrPage: React.ComponentType<any> | undefined = layoutOrPageMod
? interopDefault(layoutOrPageMod)
? await interopDefault(layoutOrPageMod)
: undefined

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/shared/lib/lazy-dynamic/loadable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ function convertModule<P>(
// Cases:
// mod: { default: Component }
// mod: Component
// mod: { default: proxy(Component) }
// mod: { $$typeof, default: proxy(Component) }
// mod: proxy(Component)
const hasDefault = mod && 'default' in mod
return {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { nextTestSetup } from 'e2e-utils'
import { FileRef, nextTestSetup } from 'e2e-utils'
import path from 'path'

describe('referencing a client component in an app route', () => {
const { next } = nextTestSetup({
files: __dirname,
files: new FileRef(path.join(__dirname)),
})

it('responds without error', async () => {
expect(JSON.parse(await next.render('/runtime'))).toEqual({
clientComponent: 'function',
myModuleClientComponent: 'function',
// Turbopack's proxy components are functions
clientComponent: process.env.TURBOPACK ? 'function' : 'object',
myModuleClientComponent: process.env.TURBOPACK ? 'function' : 'object',
})
})
})
2 changes: 1 addition & 1 deletion test/e2e/app-dir/dynamic/app/dynamic/named-export/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import dynamic from 'next/dynamic'

const Button = dynamic(() =>
import('./client').then((mod) => {
return { default: mod.Button }
return mod.Button
})
)

Expand Down

0 comments on commit a882e6e

Please sign in to comment.