Skip to content

Commit

Permalink
Fix package resolution issue in app dir (#43349)
Browse files Browse the repository at this point in the history
The server and client compilers have different external dependency resolution logic. The server prefers CJS while the client prefers ESM by default. This causes an issue when that dependency is a client component entry. We get the module reference during RSC render, which is the CJS file path. And that module reference can't be located in the flight manifest as it was generated via the client compiler (has the ESM path).

This PR changes the entry creation logic to use resolved requests, instead of the raw requests. So both SSR and client will have the same resolved import paths to ensure the module reference consistency.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
  • Loading branch information
shuding committed Nov 25, 2022
1 parent bcb0e28 commit e650047
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 16 deletions.
23 changes: 7 additions & 16 deletions packages/next/build/webpack/plugins/flight-client-entry-plugin.ts
Expand Up @@ -12,7 +12,7 @@ import {
entries,
EntryTypes,
} from '../../../server/dev/on-demand-entry-handler'
import { APP_DIR_ALIAS, WEBPACK_LAYERS } from '../../../lib/constants'
import { WEBPACK_LAYERS } from '../../../lib/constants'
import {
APP_CLIENT_INTERNALS,
COMPILER_NAMES,
Expand Down Expand Up @@ -379,23 +379,14 @@ export class FlightClientEntryPlugin {
compilation.moduleGraph.getResolvedModule(dependencyToFilter)
if (!mod) return

// Keep client imports as simple
// native or installed js module: -> raw request, e.g. next/head
// client js or css: -> user request
const rawRequest = mod.rawRequest
// Request could be undefined or ''
if (!rawRequest) return

const isCSS = regexCSS.test(rawRequest)
const isLocal =
!isCSS &&
!rawRequest.startsWith('.') &&
!rawRequest.startsWith('/') &&
!rawRequest.startsWith(APP_DIR_ALIAS)

const modRequest: string | undefined = isLocal
? rawRequest
: mod.resourceResolveData?.path + mod.resourceResolveData?.query

// We have to always use the resolved request here to make sure the
// server and client are using the same module path (required by RSC), as
// the server compiler and client compiler have different resolve configs.
const modRequest: string | undefined =
mod.resourceResolveData?.path + mod.resourceResolveData?.query

// Ensure module is not walked again if it's already been visited
if (!visitedBySegment[entryRequest]) {
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/app-dir/app-external.test.ts
Expand Up @@ -147,6 +147,11 @@ describe('app dir - external dependency', () => {
).toBe('rgb(255, 0, 0)')
})

it('should use the same export type for packages in both ssr and client', async () => {
const browser = await webdriver(next.url, '/client-dep')
expect(await browser.eval(`window.document.body.innerText`)).toBe('hello')
})

it('should handle external css modules in pages', async () => {
const browser = await webdriver(next.url, '/test-pages')

Expand Down
9 changes: 9 additions & 0 deletions test/e2e/app-dir/app-external/app/client-dep/page.js
@@ -0,0 +1,9 @@
import Foo from 'client-module'

export default function Index() {
return (
<div>
<Foo />
</div>
)
}
@@ -0,0 +1,3 @@
'use client'

module.exports = () => 'hello'
@@ -0,0 +1,3 @@
'use client'

export default () => 'hello'
@@ -0,0 +1,5 @@
{
"name": "client-module",
"main": "index.js",
"module": "index.mjs"
}

0 comments on commit e650047

Please sign in to comment.