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

Disable esm resolving for appDir and alias react #41687

Merged
merged 12 commits into from Oct 24, 2022
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