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.

Sign up for GitHub

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

Fix esm property def in flight loader #66286

Merged
merged 10 commits into from
Jun 10, 2024
Merged

Fix esm property def in flight loader #66286

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5a5396e
Fix esm property def in flight loader
huozhi May 28, 2024
a3aafc6
update test
huozhi May 29, 2024
ff4bbf8
fix barrel optmization case
huozhi May 29, 2024
37906de
update tests case
huozhi Jun 3, 2024
1b3cbe9
update default
huozhi Jun 4, 2024
4c455fb
revert
huozhi Jun 4, 2024
43e8604
remove extra await and use proxy.default
huozhi Jun 4, 2024
f3287aa
revert
huozhi Jun 4, 2024
d8a4a51
Merge branch 'canary' into fix/esm-client-ref
huozhi Jun 5, 2024
50b59db
Merge branch 'canary' into fix/esm-client-ref
huozhi Jun 8, 2024
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
17 changes: 10 additions & 7 deletions packages/next/src/build/webpack/loaders/next-flight-loader/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,21 +97,24 @@ 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.
let esmSource = `\
import { createProxy } from "${MODULE_PROXY_PATH}"

const proxy = createProxy(String.raw\`${resourceKey}\`)
`
let cnt = 0
for (const ref of clientRefs) {
if (ref === '') {
esmSource += `\nexports[''] = createProxy(String.raw\`${resourceKey}#\`);`
esmSource += `exports[''] = proxy['']\n`
} else if (ref === 'default') {
esmSource += `\
export default createProxy(String.raw\`${resourceKey}#default\`);
`
esmSource += `export default createProxy(String.raw\`${resourceKey}#default\`)\n`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why isn't this export default proxy?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found one case cannot pass, which is esm externals. When a client component became a promise because having async dependencies.

When accessing the layout or page component, we access the .default property which is the deep proxy to a Promise. React cannot handle it as a component.
Seems CJS deep proxy cannot handle this case unless we wrap one more layer.

The workaround for it now is: Still wrap one more layer for default property. We're handling it with an await here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gnoff I made it work to use proxy.default in #66990

} else {
esmSource += `
const e${cnt} = createProxy(String.raw\`${resourceKey}#${ref}\`);
export { e${cnt++} as ${ref} };`
esmSource += `const e${cnt} = proxy["${ref}"];\n`
esmSource += `export { e${cnt++} as ${ref} };\n`
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/shared/lib/lazy-dynamic/loadable.tsx
View file
Open in desktop
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: { $$typeof, default: proxy(Component) }
// mod: { default: proxy(Component) }
// mod: proxy(Component)
const hasDefault = mod && 'default' in mod
return {
Expand Down
14 changes: 4 additions & 10 deletions test/e2e/app-dir/app-routes-client-component/app-routes-client-component.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
import { FileRef, nextTestSetup } from 'e2e-utils'
import path from 'path'
import { nextTestSetup } from 'e2e-utils'

describe('referencing a client component in an app route', () => {
const { next } = nextTestSetup({
files: new FileRef(path.join(__dirname)),
dependencies: {
react: '19.0.0-rc-f994737d14-20240522',
'react-dom': '19.0.0-rc-f994737d14-20240522',
},
files: __dirname,
})

it('responds without error', async () => {
expect(JSON.parse(await next.render('/runtime'))).toEqual({
// Turbopack's proxy components are functions
clientComponent: process.env.TURBOPACK ? 'function' : 'object',
myModuleClientComponent: process.env.TURBOPACK ? 'function' : 'object',
clientComponent: 'function',
myModuleClientComponent: 'function',
})
})
})
2 changes: 1 addition & 1 deletion test/e2e/app-dir/dynamic/app/dynamic/named-export/page.js
View file
Open in desktop
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 mod.Button
return { default: mod.Button }
})
)

Expand Down
Loading