fix(perf): use swc for minify by SukkaW · Pull Request #1258 · vercel/ncc

SukkaW · 2025-03-13T13:45:41Z

Unlike #777 and #926 where swc is used to replace ts-loader, in this PR swc is used to replace terser in the modification phase.

All CI passed: SukkaW#2

socket-security · 2025-03-13T13:47:24Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub ↗.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@swc/wasm@1.11.20

View full report ↗

styfle · 2025-03-13T16:04:00Z

test/unit/minify-sourcemap-register/output.js

@@ -1,2 +1 @@
 require('./sourcemap-register.js');(()=>{if(typeof __nccwpck_require__!=="undefined")__nccwpck_require__.ab=__dirname+"/";var e={};(()=>{var _=e;const o="qux";console.log?.("hello");_.foobar=o})();module.exports=e})();
-//# sourceMappingURL=index.js.map


This looks like a bug since the source map comment is missing

Seems to be a limitation of swc as it is impossible to pass an object to the minify.sourceMap option.

ncc/src/index.js

Lines 485 to 489 in 36a7568

sourceMap: map ? {

content: map,

filename,

url: `${filename}.map`

} : false

cc @kdy1 @magic-akari

Should SWC write //# sourceMappingURL=index.js.map?

https://github.com/swc-project/swc/blob/a53c60db18bd90539f3449348cbb166f32f46223/crates/swc_compiler_base/src/lib.rs#L259-L271

SWC has relevant code for sourceMap = inline but not for true. Does terser emit one?

https://terser.org/docs/api-reference/#source-map-options so there was an option. I'll implement this option today or tomorrow and publish a new version of @swc/core

swc-project/swc#10346 implements the option, and I'll publish a new version later today, after merging some PRs.

Sorry for the late response! The mention email was buried due to lots of other emails. (Mostly PR review requests)

Sure. Lemme see if the tests passed here after bumping the swc to the latest version later today.

https://github.com/swc-project/swc/actions/runs/14376924724 failed, so I'll fix it tomorrow and retry

A new version is published! Can you try updating @swc/core?

A new version is published! Can you try updating @swc/core?

@kdy1 Sure! I will try it out later today.

@kdy1 Just bump the swc to the latest version, and it appears to be working, thanks for fixing this! cc @styfle

See also CI test runs: SukkaW#2

styfle · 2025-03-13T16:06:26Z

Unlike #777 and #926 where swc is used to replace ts-loader, in this PR swc is used to replace terser in the modification phase

I think this is only worth doing if we also plan to replace swc for the ts-loader phase too.

Otherwise we include a massive dependency that we're not taking advantage of.

Also did you compare performance before/after?

SukkaW · 2025-03-14T01:25:11Z

Otherwise we include a massive dependency that we're not taking advantage of.

We can always do this gradually since #926 has been blocked and simply abandoned since (no one is investigating it anymore).

Also did you compare performance before/after?

I use the integration test case (yarn build && time yarn test test/integration.test.js) to compare the performance.

Before: yarn test test/integration.test.js 167.13s user 15.04s system 246% cpu 1:14.01 total
@swc/wasm: yarn test test/integration.test.js 150.22s user 15.37s system 234% cpu 1:01.04 total
@swc/core: yarn test test/integration.test.js 149.66s user 15.27s system 249% cpu 1:00.60 total

However integration test cases use many packages anyway, thus I assume the performance improvement is negligible on small projects with fewer dependencies.

**Related issue:** - vercel/ncc#1258

styfle · 2025-04-14T19:39:16Z

test/unit/minify-sourcemap-register/output.js.map

@@ -1 +1 @@
-{"version":3,"file":"index.js","names":["__webpack_require__","ab","__dirname","foobar","console","log","exports"],"sources":["../webpack/runtime/compat","../test/unit/minify-sourcemap-register/input.js"],"sourcesContent":["\nif (typeof __webpack_require__ !== 'undefined') __webpack_require__.ab = __dirname + \"/\";","const foobar = 'qux'\nconsole.log?.(\"hello\");\nexports.foobar = foobar\n"],"mappings":"MACA,UAAAA,sBAAA,YAAAA,oBAAAC,GAAAC,UAAA,I,uBCDA,MAAAC,EAAA,MACAC,QAAAC,MAAA,SACAC,EAAAH,Q","ignoreList":[]}
+{"version":3,"file":"index.js","sources":["../webpack/runtime/compat",".././test/unit/minify-sourcemap-register/input.js"],"sourceRoot":"","sourcesContent":["\nif (typeof __webpack_require__ !== 'undefined') __webpack_require__.ab = __dirname + \"/\";","const foobar = 'qux'\nconsole.log?.(\"hello\");\nexports.foobar = foobar\n"],"names":[],"mappings":"MACA,GAAA,OAAA,sBAAA,YAAA,oBAAA,EAAA,CAAA,UAAA,2BCDA,MAAA,EAAA,MACA,QAAA,GAAA,GAAA,SACA,EAAA,MAAA,CAAA"}


@kdy1 Is it expected that names changed to an empty array?

Before:

["__webpack_require__","ab","__dirname","foobar","console","log","exports"]

After:

[]

That looks like a bug.

I think it's swc-project/swc#6726

It's now fixed. Can you try updating @swc/cli?

perf: use swc

1db7c46

SukkaW requested review from Timer and styfle as code owners March 13, 2025 13:45

styfle reviewed Mar 13, 2025

View reviewed changes

kdy1 mentioned this pull request Apr 10, 2025

feat(es/codegen): Support sourceMap.url option of terser swc-project/swc#10346

Merged

kdy1 added a commit to swc-project/swc that referenced this pull request Apr 10, 2025

feat(es/codegen): Support sourceMap.url option of terser (#10346)

566bc7c

**Related issue:** - vercel/ncc#1258

styfle changed the title ~~perf: use swc for minify~~ fix(perf): use swc for minify Apr 11, 2025

chore: update test case

02e95a4

SukkaW force-pushed the use-swc-minify branch from 8d8724c to 94e2073 Compare April 12, 2025 11:21

chore: update swc to the latest version

3ad9bc4

SukkaW force-pushed the use-swc-minify branch from 94e2073 to 3ad9bc4 Compare April 12, 2025 11:22

styfle reviewed Apr 14, 2025

View reviewed changes

		@@ -1,2 +1 @@
		require('./sourcemap-register.js');(()=>{if(typeof __nccwpck_require__!=="undefined")__nccwpck_require__.ab=__dirname+"/";var e={};(()=>{var _=e;const o="qux";console.log?.("hello");_.foobar=o})();module.exports=e})();
		//# sourceMappingURL=index.js.map

	sourceMap: map ? {
	content: map,
	filename,
	url: `${filename}.map`
	} : false

		@@ -1 +1 @@
		{"version":3,"file":"index.js","names":["__webpack_require__","ab","__dirname","foobar","console","log","exports"],"sources":["../webpack/runtime/compat","../test/unit/minify-sourcemap-register/input.js"],"sourcesContent":["\nif (typeof __webpack_require__ !== 'undefined') __webpack_require__.ab = __dirname + \"/\";","const foobar = 'qux'\nconsole.log?.(\"hello\");\nexports.foobar = foobar\n"],"mappings":"MACA,UAAAA,sBAAA,YAAAA,oBAAAC,GAAAC,UAAA,I,uBCDA,MAAAC,EAAA,MACAC,QAAAC,MAAA,SACAC,EAAAH,Q","ignoreList":[]} No newline at end of file
		{"version":3,"file":"index.js","sources":["../webpack/runtime/compat",".././test/unit/minify-sourcemap-register/input.js"],"sourceRoot":"","sourcesContent":["\nif (typeof __webpack_require__ !== 'undefined') __webpack_require__.ab = __dirname + \"/\";","const foobar = 'qux'\nconsole.log?.(\"hello\");\nexports.foobar = foobar\n"],"names":[],"mappings":"MACA,GAAA,OAAA,sBAAA,YAAA,oBAAA,EAAA,CAAA,UAAA,2BCDA,MAAA,EAAA,MACA,QAAA,GAAA,GAAA,SACA,EAAA,MAAA,CAAA"}

Conversation

SukkaW commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

socket-security bot commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SukkaW Mar 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

styfle commented Mar 13, 2025

Uh oh!

SukkaW commented Mar 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SukkaW commented Mar 13, 2025 •

edited

Loading

socket-security bot commented Mar 13, 2025 •

edited

Loading

SukkaW Mar 14, 2025 •

edited

Loading