Skip to content

Commit

Permalink
Disable esm resolving for appDir and alias react (#41687)
Browse files Browse the repository at this point in the history
When esm package that depending on react is used between `pages/` and
`app/`, it's loading the user installed react through esm loader which
doesn't go through require hook, so we cannot intercept it with
require-hook.

Disable esm resolving when `appDir` is enabled for now, prefering to
pick the cjs bundle so that react import is intercepted by require hook
and pointing to the built-in react

Co-authored-by: JJ Kasper <jj@jjsweb.site>
  • Loading branch information
huozhi and ijjk committed Oct 24, 2022
1 parent 48c7a80 commit 1fa0068
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 13 deletions.
24 changes: 23 additions & 1 deletion packages/next/build/index.ts
Expand Up @@ -302,9 +302,31 @@ export default async function build(

const publicDir = path.join(dir, 'public')
const isAppDirEnabled = !!config.experimental.appDir
const initialRequireHookFilePath = require.resolve(
'next/dist/server/initialize-require-hook'
)
const content = await promises.readFile(
initialRequireHookFilePath,
'utf8'
)

if (isAppDirEnabled) {
process.env.NEXT_PREBUNDLED_REACT = '1'
}
await promises
.writeFile(
initialRequireHookFilePath,
content.replace(
/isPrebundled = (true|false)/,
`isPrebundled = ${isAppDirEnabled}`
)
)
.catch((err) => {
if (isAppDirEnabled) {
throw err
}
})

const { pagesDir, appDir } = findPagesDir(dir, isAppDirEnabled)
const hasPublicDir = await fileExists(publicDir)

Expand Down Expand Up @@ -342,7 +364,7 @@ export default async function build(
Log.info('Skipping validation of types')
}
if (runLint && ignoreESLint) {
// only print log when build requre lint while ignoreESLint is enabled
// only print log when build require lint while ignoreESLint is enabled
Log.info('Skipping linting')
}

Expand Down
23 changes: 19 additions & 4 deletions packages/next/build/webpack-config.ts
Expand Up @@ -130,9 +130,9 @@ function isResourceInPackages(resource: string, packageNames?: string[]) {

const bundledReactImports = [
'react',
'react-dom',
'react/jsx-runtime',
'react/jsx-dev-runtime',
// 'react-dom',
'next/dist/compiled/react-server-dom-webpack/server.browser',
]

Expand Down Expand Up @@ -440,11 +440,12 @@ export const nextImageLoaderRegex =
/\.(png|jpg|jpeg|gif|webp|avif|ico|bmp|svg)$/i

export async function resolveExternal(
appDir: string,
dir: string,
esmExternalsConfig: NextConfigComplete['experimental']['esmExternals'],
context: string,
request: string,
isEsmRequested: boolean,
hasAppDir: boolean,
getResolve: (
options: any
) => (
Expand All @@ -466,6 +467,11 @@ export async function resolveExternal(

let preferEsmOptions =
esmExternals && isEsmRequested ? [true, false] : [false]
// Disable esm resolving for app/ and pages/ so for esm package using under pages/
// won't load react through esm loader
if (hasAppDir) {
preferEsmOptions = [false]
}
for (const preferEsm of preferEsmOptions) {
const resolve = getResolve(
preferEsm ? esmResolveOptions : nodeResolveOptions
Expand Down Expand Up @@ -505,7 +511,7 @@ export async function resolveExternal(
const baseResolve = getResolve(
isEsm ? baseEsmResolveOptions : baseResolveOptions
)
;[baseRes, baseIsEsm] = await baseResolve(appDir, request)
;[baseRes, baseIsEsm] = await baseResolve(dir, request)
} catch (err) {
baseRes = null
baseIsEsm = false
Expand Down Expand Up @@ -1063,9 +1069,16 @@ export default async function getBaseWebpackConfig(
// Absolute requires (require('/foo')) are extremely uncommon, but
// also have no need for customization as they're already resolved.
if (!isLocal) {
if (/^(?:next$|react(?:$|\/)|react-dom(?:$|\/))/.test(request)) {
if (/^(?:next$)/.test(request)) {
return `commonjs ${request}`
}
if (/^(react(?:$|\/)|react-dom(?:$|\/))/.test(request)) {
// override react-dom to server-rendering-stub for server
if (request === 'react-dom' && hasAppDir && !isClient) {
request = 'react-dom/server-rendering-stub'
}
return `commonjs ${hasAppDir ? 'next/dist/compiled/' : ''}${request}`
}

const notExternalModules =
/^(?:private-next-pages\/|next\/(?:dist\/pages\/|(?:app|document|link|image|legacy\/image|constants|dynamic|script|navigation|headers)$)|string-hash|private-next-rsc-mod-ref-proxy$)/
Expand All @@ -1082,6 +1095,7 @@ export default async function getBaseWebpackConfig(

// When in esm externals mode, and using import, we resolve with
// ESM resolving options.
// Also disable esm request when appDir is enabled
const isEsmRequested = dependencyType === 'esm'

const isLocalCallback = (localRes: string) => {
Expand Down Expand Up @@ -1121,6 +1135,7 @@ export default async function getBaseWebpackConfig(
context,
request,
isEsmRequested,
hasAppDir,
getResolve,
isLocal ? isLocalCallback : undefined
)
Expand Down
Expand Up @@ -643,6 +643,7 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {
context,
request,
isEsmRequested,
!!this.appDirEnabled,
(options) => (_: string, resRequest: string) => {
return getResolve(options)(parent, resRequest, job)
},
Expand Down
12 changes: 12 additions & 0 deletions packages/next/server/initialize-require-hook.ts
@@ -0,0 +1,12 @@
import {
loadRequireHook,
overrideBuiltInReactPackages,
} from '../build/webpack/require-hook'

loadRequireHook()

const isPrebundled = false

if (isPrebundled) {
overrideBuiltInReactPackages()
}
5 changes: 1 addition & 4 deletions packages/next/server/next-server.ts
@@ -1,3 +1,4 @@
import './initialize-require-hook'
import './node-polyfill-fetch'
import './node-polyfill-web-streams'

Expand Down Expand Up @@ -97,17 +98,13 @@ import { shouldUseReactRoot } from './utils'
import ResponseCache from './response-cache'
import { IncrementalCache } from './lib/incremental-cache'
import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'
import { loadRequireHook } from '../build/webpack/require-hook'

if (shouldUseReactRoot) {
;(process.env as any).__NEXT_REACT_ROOT = 'true'
}

import { renderToHTMLOrFlight as appRenderToHTMLOrFlight } from './app-render'

// require hook for custom server
loadRequireHook()

export * from './base-server'

type ExpressMiddleware = (
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/app/pages/index.js
@@ -1,13 +1,16 @@
import React from 'react'
import Link from 'next/link'
import useSWR from 'swr'
import styles from '../styles/shared.module.css'

export default function Page(props) {
const { data } = useSWR('swr-index', (v) => v, { fallbackData: 'swr-index' })
return (
<>
<b>rc:{React.Component ? 'c' : 'no'}</b>
<p className={styles.content}>hello from pages/index</p>
<Link href="/dashboard">Dashboard</Link>
<div>{data}</div>
</>
)
}
8 changes: 6 additions & 2 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -28,8 +28,9 @@ describe('app dir', () => {
next = await createNext({
files: new FileRef(path.join(__dirname, 'app')),
dependencies: {
react: 'experimental',
'react-dom': 'experimental',
swr: '2.0.0-rc.0',
react: 'latest',
'react-dom': 'latest',
sass: 'latest',
},
skipStart: true,
Expand Down Expand Up @@ -118,6 +119,9 @@ describe('app dir', () => {
it('should serve from pages', async () => {
const html = await renderViaHTTP(next.url, '/')
expect(html).toContain('hello from pages/index')

// esm imports should work fine in pages/
expect(html).toContain('swr-index')
})

it('should serve dynamic route from pages', async () => {
Expand Down
@@ -0,0 +1,6 @@
Object.defineProperty(exports, '__esModule', { value: true })
exports.default = () => 'esm-export'
exports.named = 'named'
exports.value = 123
exports.array = [4, 5, 6]
exports.obj = { x: 1 }
@@ -1,6 +1,8 @@
{
"name": "non-isomorphic-text",
"type": "module",
"exports": "./index.mjs",
"exports": {
"import": "./index.mjs",
"require": "./index.js"
},
"browser": "./browser.js"
}
1 change: 1 addition & 0 deletions test/e2e/app-dir/vercel-analytics.test.ts
Expand Up @@ -19,6 +19,7 @@ describe('vercel analytics', () => {
next = await createNext({
files: new FileRef(path.join(__dirname, 'app')),
dependencies: {
swr: '2.0.0-rc.0',
react: 'experimental',
'react-dom': 'experimental',
sass: 'latest',
Expand Down

0 comments on commit 1fa0068

Please sign in to comment.