Skip to content

Commit

Permalink
Ensure @swc/helpers do not rely on hoisting (#38174)
Browse files Browse the repository at this point in the history
This ensures we properly alias and internalize `@swc/helpers` so that we don't rely on package managers hoisting this dependency for the build to work properly. This also disables the external helpers with `jest` as this can also require hoisting to work and doesn't provide as much of an optimization. 

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

Fixes: [slack thread](https://vercel.slack.com/archives/C0289CGVAR2/p1656437059151439)
  • Loading branch information
ijjk committed Jun 29, 2022
1 parent 66b83f4 commit 8a5c59b
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 2 deletions.
2 changes: 1 addition & 1 deletion packages/next/build/swc/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function getBaseSWCOptions({
paths,
}
: {}),
externalHelpers: !process.versions.pnp,
externalHelpers: !process.versions.pnp && !jest,
parser: parserConfig,
experimental: {
keepImportAssertions: true,
Expand Down
10 changes: 10 additions & 0 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,10 @@ export default async function getBaseWebpackConfig(
}
: {}),

'@swc/helpers': path.dirname(
require.resolve('@swc/helpers/package.json')
),

setimmediate: 'next/dist/compiled/setimmediate',
},
...(isClient || isEdgeServer
Expand Down Expand Up @@ -816,6 +820,12 @@ export default async function getBaseWebpackConfig(
}
}

// @swc/helpers should not be external as it would
// require hoisting the package which we can't rely on
if (request.includes('@swc/helpers')) {
return
}

// When in esm externals mode, and using import, we resolve with
// ESM resolving options.
const isEsmRequested = dependencyType === 'esm'
Expand Down
38 changes: 38 additions & 0 deletions test/e2e/handle-non-hoisted-swc-helpers/index.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { createNext } from 'e2e-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { renderViaHTTP } from 'next-test-utils'

describe('handle-non-hoisted-swc-helpers', () => {
let next: NextInstance

beforeAll(async () => {
next = await createNext({
files: {
'pages/index.js': `
export default function Page() {
return <p>hello world</p>
}
export function getServerSideProps() {
const helper = require('@swc/helpers/lib/_object_spread.js')
console.log(helper)
return {
props: {
now: Date.now()
}
}
}
`,
},
installCommand:
'npm install; mkdir -p node_modules/next/node_modules/@swc; mv node_modules/@swc/helpers node_modules/next/node_modules/@swc/',
dependencies: {},
})
})
afterAll(() => next.destroy())

it('should work', async () => {
const html = await renderViaHTTP(next.url, '/')
expect(html).toContain('hello world')
})
})
2 changes: 1 addition & 1 deletion test/lib/create-next-install.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ async function createNextInstall(
if (!(packageJson && packageJson.nextPrivateSkipLocalDeps)) {
const pkgPaths = await linkPackages(tmpRepoDir)
combinedDependencies = {
next: pkgPaths.get('next'),
...Object.keys(dependencies).reduce((prev, pkg) => {
const pkgPath = pkgPaths.get(pkg)
prev[pkg] = pkgPath || dependencies[pkg]
return prev
}, {}),
next: pkgPaths.get('next'),
}
}

Expand Down

0 comments on commit 8a5c59b

Please sign in to comment.