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.

Sign up for GitHub

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

Enable externalHelpers when pre compiling Next.js' code #37164

Merged
merged 4 commits into from
Jun 19, 2022
Merged

Enable externalHelpers when pre compiling Next.js' code #37164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1c47f4f
chore: enable `externalHelpers` for pre-compile
SukkaW Jun 16, 2022
bf228dd
chore(devDeps): update @swc/[email protected]
SukkaW Jun 16, 2022
e0ffd8b
Merge branch 'canary' into external-helper-build
SukkaW Jun 17, 2022
90f359d
Merge branch 'canary' into external-helper-build
kodiakhq[bot] Jun 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"@next/swc": "workspace:*",
"@svgr/webpack": "5.5.0",
"@swc/cli": "0.1.55",
"@swc/core": "1.2.148",
"@swc/core": "1.2.203",
"@swc/helpers": "0.3.17",
"@testing-library/react": "13.0.0",
"@types/cheerio": "0.22.16",
Expand Down
8 changes: 6 additions & 2 deletions packages/next/taskfile-swc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ module.exports = function (task) {

const isClient = serverOrClient === 'client'

/** @type {import('@swc/core').Options} */
const swcClientOptions = {
module: {
type: 'commonjs',
ignoreDynamic: true,
},
jsc: {
loose: true,

externalHelpers: true,
Copy link
Member

@ijjk ijjk Jun 18, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could use the next-swc binary here for transforming instead of installing @swc/core as well, this could help us ensure we're matching behavior as this is already enabled there.

This would also allow us to remove @swc/core as a dependency of the monorepo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. It also helps eliminate the inconsistency. Let me see what I can do.

Copy link
Contributor Author

@SukkaW SukkaW Jun 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ijjk I have tried to re-use the file next/build/swc/index.js, but the file is dependent on the precompiled @napi-rs/triple.

Maybe it could be done in another PR? Where the shared logic can be extracted into @next/swc package.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could potentially expose an env variable to signal it should use the direct module instead of the compiled one e.g. if (process.env.__NEXT_TASKR) { require('@napi-rs/triple') }

Yeah we can definitely investigate this in a separate PR

target: 'es2016',
parser: {
syntax: 'typescript',
Expand All @@ -55,6 +56,7 @@ module.exports = function (task) {
},
}

/** @type {import('@swc/core').Options} */
const swcServerOptions = {
module: {
type: 'commonjs',
Expand All @@ -67,7 +69,9 @@ module.exports = function (task) {
},
jsc: {
loose: true,

// Do not enable externalHelpers for server-side code
// "_is_native_function.mjs" helper is not compatible with edge runtime
externalHelpers: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this is also an issue with the client x-ref: #37829

parser: {
syntax: 'typescript',
dynamicImport: true,
Expand Down
4 changes: 2 additions & 2 deletions packages/next/taskfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1956,7 +1956,7 @@ export async function shared(task, opts) {
.source(
opts.src || 'shared/**/!(amp|config|constants|dynamic|head).+(js|ts|tsx)'
)
.swc('server', { dev: opts.dev })
.swc('client', { dev: opts.dev })
.target('dist/shared')
notify('Compiled shared files')
}
Expand All @@ -1966,7 +1966,7 @@ export async function shared_re_exported(task, opts) {
.source(
opts.src || 'shared/**/{amp,config,constants,dynamic,head}.+(js|ts|tsx)'
)
.swc('server', { dev: opts.dev, interopClientDefaultExport: true })
.swc('client', { dev: opts.dev, interopClientDefaultExport: true })
.target('dist/shared')
notify('Compiled shared re-exported files')
}
Expand Down
92 changes: 46 additions & 46 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.