chore: bump oxc to v0.21.0 #1656

Dunqing · 2024-07-18T03:17:51Z

Description

The changelog is here. Most of the snapshots changed due to codegen printing double quotes by default.

Also, in this version, we have aligned the ts ast scope with TypeScript. Many ignored ts tests may have been resolved.

netlify · 2024-07-18T03:18:25Z

✅ Deploy Preview for rolldown-rs canceled.

Name	Link
🔨 Latest commit	`2ae1567`
🔍 Latest deploy log	https://app.netlify.com/sites/rolldown-rs/deploys/6698cf6c8ff1190008c7f1ad

crates/rolldown/tests/fixtures/format/iife/basic/artifacts.snap

crates/rolldown/tests/fixtures/deconflict/wrapped_esm_default_function/artifacts.snap

crates/rolldown/tests/fixtures/deconflict/decl_nested_assign_pattern/artifacts.snap

github-actions · 2024-07-18T03:44:42Z

Benchmarks Rust

target: main(68ee262)
pr: bump/oxc-0.21.0(2ae1567)

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     47.5±0.61ms        ? ?/sec    1.05     49.7±1.19ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     58.0±1.20ms        ? ?/sec    1.03     59.9±0.99ms        ? ?/sec
bundle/bundle@rome-ts                                        1.00     91.7±1.09ms        ? ?/sec    1.03     94.8±1.17ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                              1.00    110.8±2.66ms        ? ?/sec    1.02    113.0±1.27ms        ? ?/sec
bundle/bundle@threejs                                        1.00     27.3±0.60ms        ? ?/sec    1.01     27.6±0.82ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.01     37.1±0.67ms        ? ?/sec    1.00     36.7±0.72ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    306.3±5.97ms        ? ?/sec    1.01    307.9±3.87ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    396.7±6.59ms        ? ?/sec    1.00    396.1±6.00ms        ? ?/sec
remapping/remapping                                          1.02     50.0±0.53ms        ? ?/sec    1.00     49.2±0.14ms        ? ?/sec
remapping/render-chunk-remapping                             1.06    116.5±0.39ms        ? ?/sec    1.00    109.9±0.78ms        ? ?/sec
scan/scan@rome-ts                                            1.00     72.1±0.78ms        ? ?/sec    1.07     77.3±0.79ms        ? ?/sec
scan/scan@rome-ts-sourcemap                                  1.00     73.4±0.63ms        ? ?/sec    1.05     77.2±1.08ms        ? ?/sec
scan/scan@threejs                                            1.00     20.4±0.31ms        ? ?/sec    1.01     20.6±0.46ms        ? ?/sec
scan/scan@threejs-sourcemap                                  1.00     19.9±0.18ms        ? ?/sec    1.05     20.9±0.25ms        ? ?/sec
scan/scan@threejs10x                                         1.00    204.5±1.98ms        ? ?/sec    1.04    212.3±3.09ms        ? ?/sec
scan/scan@threejs10x-sourcemap                               1.00    203.6±1.79ms        ? ?/sec    1.04    212.7±2.13ms        ? ?/sec

crates/rolldown/tests/fixtures/cjs_compat/reexport_commonjs/artifacts.snap

crates/rolldown/tests/esbuild/loader/auto_detect_mime_type_from_extension/artifacts.snap

codspeed-hq · 2024-07-18T03:48:01Z

CodSpeed Performance Report

Merging #1656 will improve performances by 6.1%

_{Comparing bump/oxc-0.21.0 (2ae1567) with main (68ee262)}

Summary

⚡ 3 improvements
✅ 13 untouched benchmarks

Benchmarks breakdown

	Benchmark	`main`	`bump/oxc-0.21.0`	Change
⚡	`bundle@rome-ts`	499.8 ms	475.9 ms	+5.02%
⚡	`scan@rome-ts`	415.4 ms	391.6 ms	+6.1%
⚡	`scan@rome-ts-sourcemap`	414.9 ms	391.9 ms	+5.86%

crates/rolldown/tests/esbuild/splitting/dynamic_and_not_dynamic_es6_into_es6/artifacts.snap

IWANABETHATGUY · 2024-07-18T03:50:33Z

crates/rolldown/tests/esbuild/lower/lower_private_method2020/artifacts.snap

-		c()?.foo(x);
-		d().foo?.(x);
-		e()?.foo?.(x);
+		(a()).foo;


crates/rolldown/tests/esbuild/dce/disable_tree_shaking/artifacts.snap

IWANABETHATGUY · 2024-07-18T03:58:19Z

Overall it looks good except some bad cases in call expr chain and ObjectExpr with shorthand, could you track all the bad cases upstream?

Cargo.toml

Dunqing · 2024-07-18T05:19:40Z

The node tests are difficult to find out why it fails, and I'm wondering why we don't print out the Vitest error.

hyf0 · 2024-07-18T05:22:40Z

The node tests are difficult to find out why it fails, and I'm wondering why we don't print out the Vitest error.

Looks like tests on vitest are all passed. It's just one failed in rollup tests: https://github.com/rolldown/rolldown/actions/runs/9985933904/job/27597619752?pr=1656.

No git URL on cargo.toml

Dunqing · 2024-07-18T05:24:14Z

Looks like tests on vitest are all passed. It's just one failed in rollup tests: https://github.com/rolldown/rolldown/actions/runs/9985933904/job/27597619752?pr=1656.

Please see https://github.com/rolldown/rolldown/actions/runs/9985756953/job/27597155926

hyf0 · 2024-07-18T05:26:08Z

Looks like tests on vitest are all passed. It's just one failed in rollup tests: https://github.com/rolldown/rolldown/actions/runs/9985933904/job/27597619752?pr=1656.

Please see https://github.com/rolldown/rolldown/actions/runs/9985756953/job/27597155926

Error: Failed in ./fixtures/output/hashFileNames/_config.ts

Run just test-node rolldown --update for this one.

Failed in ./fixtures/tree-shake/module-side-effects-regex/_config.ts

This one needs to be investigate. I'll take look now.

Dunqing · 2024-07-18T05:29:31Z

Error: Failed in ./fixtures/output/hashFileNames/_config.ts

Run just test-node rolldown --update for this one.

Failed in ./fixtures/tree-shake/module-side-effects-regex/_config.ts

This one needs to be investigate. I'll take look now.

I already fixed it. 51701d3

hyf0 · 2024-07-18T05:36:19Z

I found the cause of the failed rollup test. Not sure how to describle it.

Dunqing · 2024-07-18T05:38:41Z

I found the cause of the failed rollup test. Not sure how to describle it.

Could you tell me if it is caused by oxc?

hyf0 · 2024-07-18T05:41:18Z

let o = {
	f() {
		assert.ok(this !== o);
	}
};
(1, o.f)();
(1, o.f)``;

(true && o.f)();
(true && o.f)``;

(true ? o.f : false)();
(true ? o.f : false)``;

is bundled to

let o = {f() {
	assert.ok(this !== o);
}};
(1, o.f)();
(1, o.f)``;
o.f();
o.f``;
o.f();
o.f``;

o.f``;

is not the same as

(true && o.f)``;

We used

rolldown/crates/rolldown/src/utils/pre_process_ecma_ast.rs

Line 52 in 401dd61

RemoveDeadCode::new(fields.allocator).build(fields.program);

to simplify expressions. I guess this will cause

(true && o.f)``;

become

(o.f)``;

which is still ~~correct and has the same semantic as~~ edited: this is also wrong, see the following output of esbuild

(1, o.f)``;

.

But this PR oxc finally printed to

o.f``;

which is wrong. Not sure if we should fix it in the RemoveDeadCode or this is a bug of oxc_codegen.

See the output of esbuld

let o={f(){assert.ok(this!==o)}};(0,o.f)(),(0,o.f)``,(0,o.f)(),(0,o.f)``,(0,o.f)(),(0,o.f)``;

https://esbuild.github.io/try/#dAAwLjIzLjAALS1taW5pZnkAbGV0IG8gPSB7CglmKCkgewoJCWFzc2VydC5vayh0aGlzICE9PSBvKTsKCX0KfTsKKDEsIG8uZikoKTsKKDEsIG8uZilgYDsKCih0cnVlICYmIG8uZikoKTsKKHRydWUgJiYgby5mKWBgOwoKKHRydWUgPyBvLmYgOiBmYWxzZSkoKTsKKHRydWUgPyBvLmYgOiBmYWxzZSlgYDs

hyf0 · 2024-07-18T05:45:42Z

This is the main branch output

let o = {f() {
	assert.ok(this !== o);
}};
(1, o.f)();
(1, o.f)``;
(true && o.f)();
(true && o.f)``;
(true ? o.f : false)();
(true ? o.f : false)``;

So it seems we passed this rollup test because we didn't support simplify these expressions before. So they keep the semantics.

hyf0 · 2024-07-18T05:47:42Z

This is a classic one. Esbuild has the issue before. evanw/esbuild#2610

Dunqing · 2024-07-18T05:53:53Z

From the description, it looks like the bug comes from RemoveDeadCode.

This is a classic one. Esbuild has the issue before. evanw/esbuild#2610

Haha, The issue author is also you.

Let's fix it in the next patch

hyf0

Nice!

github-actions bot added the needs-triage label Jul 18, 2024