Skip to content

Commit

Permalink
Fix react-refresh for transpiled packages (#60563)
Browse files Browse the repository at this point in the history
### What

We're applying react-refresh to browser layer and inject ESM or CJS
helper based on file type. Some package from `trasnpilePackages` might
contain CJS browser bundle. And injecting ESM helper breaks them.
Actually they don't need to have fast refresh ability since they're in
`node_modules`.

### How
Skip react-refresh for transpiled packages as they're in node_modules
and won't change.

Fixes #56487 
Closes NEXT-2061
  • Loading branch information
huozhi authored Jan 12, 2024
1 parent 61803f8 commit 3738e8e
Show file tree
Hide file tree
Showing 10 changed files with 26 additions and 8 deletions.
8 changes: 7 additions & 1 deletion packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -781,6 +781,12 @@ export default async function getBaseWebpackConfig(
.join('|')})[/\\\\]`
)

const transpilePackagesRegex = new RegExp(
`[/\\\\]node_modules[/\\\\](${config.transpilePackages
?.map((p) => p.replace(/\//g, '[/\\\\]'))
.join('|')})[/\\\\]`
)

const handleExternals = makeExternalHandler({
config,
optOutBundlingPackages,
Expand Down Expand Up @@ -1398,7 +1404,7 @@ export default async function getBaseWebpackConfig(
? [
{
test: codeCondition.test,
exclude: codeCondition.exclude,
exclude: [codeCondition.exclude, transpilePackagesRegex],
issuerLayer: WEBPACK_LAYERS.appPagesBrowser,
use: reactRefreshLoaders,
resolve: {
Expand Down
6 changes: 5 additions & 1 deletion test/e2e/app-dir/app-external/app-external.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createNextDescribe } from 'e2e-utils'
import { check, shouldRunTurboDevTest } from 'next-test-utils'
import { check, hasRedbox, shouldRunTurboDevTest } from 'next-test-utils'

async function resolveStreamResponse(response: any, onData?: any) {
let result = ''
Expand Down Expand Up @@ -229,6 +229,10 @@ createNextDescribe(
it('should emit cjs helpers for external cjs modules when compiled', async () => {
const $ = await next.render$('/cjs/client')
expect($('#private-prop').text()).toBe('prop')
expect($('#transpile-cjs-lib').text()).toBe('transpile-cjs-lib')

const browser = await next.browser('/cjs/client')
expect(await hasRedbox(browser)).toBe(false)
})

it('should export client module references in esm', async () => {
Expand Down
2 changes: 2 additions & 0 deletions test/e2e/app-dir/app-external/app/cjs/client/page.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
'use client'

import { instance } from 'cjs-modern-syntax'
import { packageName } from 'transpile-cjs-lib'

export default function Page() {
return (
<>
<div id="private-prop">{instance.getProp()}</div>
<div id="transpile-cjs-lib">{packageName}</div>
</>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import getType, { named, value, array, obj } from 'non-isomorphic-text'

import add from 'untranspiled-module'
import add from 'transpile-ts-lib'

// ESM externals has react has a peer dependency
import useSWR from 'swr'
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/app-dir/app-external/next.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module.exports = {
reactStrictMode: true,
transpilePackages: ['untranspiled-module', 'css', 'font'],
transpilePackages: ['css', 'font', 'transpile-ts-lib', 'transpile-cjs-lib'],
experimental: {
serverComponentsExternalPackages: [
'conditional-exports-optout',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
exports.packageName = 'transpile-cjs-lib'
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "transpile-cjs-lib",
"type": "commonjs",
"exports": "./index.js"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "transpile-ts-lib",
"main": "index.ts"
}

This file was deleted.

0 comments on commit 3738e8e

Please sign in to comment.