Skip to content

Comments

chore(rolldown): oxc v0.54.0#3725

Merged
Boshen merged 3 commits intomainfrom
next-oxc
Mar 4, 2025
Merged

chore(rolldown): oxc v0.54.0#3725
Boshen merged 3 commits intomainfrom
next-oxc

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Mar 1, 2025

No description provided.

@netlify
Copy link

netlify bot commented Mar 1, 2025 •
edited
Loading

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 10ad9d8
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/67c6756003d73000082e3471

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2025 •
edited
Loading

Benchmarks Rust

group                                                               pr                                     target
-----                                                               --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol                     1.04     78.6±1.22ms        ? ?/sec    1.00     75.6±1.69ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify              1.04     99.3±1.32ms        ? ?/sec    1.00     95.9±2.11ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify-sourcemap    1.06    113.0±2.23ms        ? ?/sec    1.00    106.8±1.36ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap           1.02     85.5±0.92ms        ? ?/sec    1.00     84.1±1.83ms        ? ?/sec
bundle/bundle@rome-ts                                               1.03    131.2±2.13ms        ? ?/sec    1.00    127.5±3.01ms        ? ?/sec
bundle/bundle@rome-ts-minify                                        1.06    227.7±8.23ms        ? ?/sec    1.00    214.6±5.16ms        ? ?/sec
bundle/bundle@rome-ts-minify-sourcemap                              1.08    267.3±7.64ms        ? ?/sec    1.00    247.4±7.89ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                                     1.04    142.6±1.68ms        ? ?/sec    1.00    136.7±1.88ms        ? ?/sec
bundle/bundle@threejs                                               1.00     44.7±1.94ms        ? ?/sec    1.01     45.3±3.53ms        ? ?/sec
bundle/bundle@threejs-minify                                        1.05     93.2±1.94ms        ? ?/sec    1.00     89.1±2.13ms        ? ?/sec
bundle/bundle@threejs-minify-sourcemap                              1.02    112.9±2.66ms        ? ?/sec    1.00    110.5±3.86ms        ? ?/sec
bundle/bundle@threejs-sourcemap                                     1.03     50.9±0.52ms        ? ?/sec    1.00     49.6±0.39ms        ? ?/sec
bundle/bundle@threejs10x                                            1.01    456.7±6.87ms        ? ?/sec    1.00    450.1±9.38ms        ? ?/sec
bundle/bundle@threejs10x-minify                                     1.01  1108.1±10.89ms        ? ?/sec    1.00   1098.9±5.48ms        ? ?/sec
bundle/bundle@threejs10x-minify-sourcemap                           1.00  1260.8±11.71ms        ? ?/sec    1.01  1275.1±10.61ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                                  1.04   529.5±17.64ms        ? ?/sec    1.00    511.5±6.33ms        ? ?/sec
remapping/remapping                                                 1.04     26.6±1.10ms        ? ?/sec    1.00     25.5±2.33ms        ? ?/sec
remapping/render-chunk-remapping                                    1.08     68.7±5.35ms        ? ?/sec    1.00     63.7±0.57ms        ? ?/sec
scan/scan@rome-ts                                                   1.03     99.6±1.51ms        ? ?/sec    1.00     96.2±1.01ms        ? ?/sec
scan/scan@threejs                                                   1.03     32.7±1.87ms        ? ?/sec    1.00     31.9±1.16ms        ? ?/sec
scan/scan@threejs10x                                                1.04    329.1±6.59ms        ? ?/sec    1.00    317.1±3.60ms        ? ?/sec

@IWANABETHATGUY
Copy link
Member

IWANABETHATGUY commented Mar 1, 2025

This issue could be closed #3245

@IWANABETHATGUY
Copy link
Member

IWANABETHATGUY commented Mar 1, 2025

This issue could be closed #3245

Ignore me, for this case:

(() => {
  const foo = () => {
  };

  /* #__PURE__ */ foo();
})();

both, foo CallExpr and VarDecl foo should be removed but the decl is preserved after minify,

@Boshen Boshen force-pushed the next-oxc branch 2 times, most recently from 2e93868 to 4a2311c Compare March 2, 2025 00:21
@Boshen
Copy link
Member Author

Boshen commented Mar 2, 2025 •
edited
Loading

@sapphi-red got a new bug, require is not checked when eliminating IIFE.

(require => require('/test.txt'))() -> require("/test.txt");

thread 'test_tests__esbuild__default__false_require__config_json' panicked at crates/rolldown_testing/src/integration_test.rs:167:11:
Expected the bundling to be success, but got diagnosable errors: BatchedBuildDiagnostic(
    [
        BuildDiagnostic {
            inner: DiagnosableResolveError {
                source: "(require => require('/test.txt'))()",
                importer_id: "/Users/boshen/github/rolldown/crates/rolldown/tests/esbuild/default/false_require/entry.js",
                importee: Span(
                    Span {
                        start: 20,
                        end: 31,
                    },
                ),
                reason: "Module not found.",
                title: Some(
                    "UNRESOLVED_IMPORT",
                ),
                help: None,
            },
            source: None,
            severity: Error,
        },
    ],
)

@sapphi-red sapphi-red mentioned this pull request Mar 2, 2025
Merged
graphite-app bot pushed a commit to oxc-project/oxc that referenced this pull request Mar 2, 2025
@sapphi-red
fix(minifier): don't inline IIFE with parameters (#9477)
9892aa2 
`(a => { a() })()` should not be inlined (requires checking default parameters).
this PR should fix the case written in rolldown/rolldown#3725 (comment) .
graphite-app bot pushed a commit to oxc-project/oxc that referenced this pull request Mar 2, 2025
@sapphi-red
fix(minifier): don't inline IIFE with parameters (#9477)
0b96ebe 
`(a => { a() })()` should not be inlined (requires checking default parameters).
this PR should fix the case written in rolldown/rolldown#3725 (comment) .
@Boshen
Copy link
Member Author

Boshen commented Mar 2, 2025 •
edited
Loading

It's almost done. Next failure is the test

rolldown/packages/rolldown/tests/fixtures/tree-shake/annotations/_config.ts

Line 8 in 1a60f39

annotations: false,

https://rollupjs.org/configuration-options/#treeshake-annotations

where we need to propagate this option to the minifier 🤔 ?

Shall we disable this test in this PR, and raise a tracking issue so we don't block everything?

@IWANABETHATGUY
Copy link
Member

IWANABETHATGUY commented Mar 2, 2025

It's almost done. Next failure is the test

rolldown/packages/rolldown/tests/fixtures/tree-shake/annotations/_config.ts

Line 8 in 1a60f39

annotations: false,

https://rollupjs.org/configuration-options/#treeshake-annotations

where we need to propagate this option to the minifier 🤔 ?

Shall we disable this test in this PR, and raise a tracking issue so we don't block everything?

LGTM

@IWANABETHATGUY
Copy link
Member

IWANABETHATGUY commented Mar 2, 2025 

import type { OutputChunk as RolldownOutputChunk } from 'rolldown'
import { defineTest } from 'rolldown-tests'
import { expect } from 'vitest'

export default defineTest({
  skip: true,
  config: {
    treeshake: {
      annotations: false,
    },
  },
  afterTest: (output) => {
    output.output
      .filter(({ type }) => type === 'chunk')
      .forEach((chunk) => {
        let code = (chunk as RolldownOutputChunk).code
        expect(code.includes(`hello`)).toBe(true)
      })
  },
})

YOu could skip the test in this way.

@Boshen Boshen force-pushed the next-oxc branch 2 times, most recently from 669675d to 7e823d4 Compare March 3, 2025 15:12
@IWANABETHATGUY IWANABETHATGUY mentioned this pull request Mar 3, 2025
Closed
@Boshen Boshen marked this pull request as ready for review March 4, 2025 01:36
@Boshen Boshen enabled auto-merge March 4, 2025 01:57
@socket-security
Copy link

socket-security bot commented Mar 4, 2025 •
edited
Loading

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@oxc-project/[email protected] ➜ 0.54.0 None 0 235 kB boshen
npm/@oxc-project/[email protected] ➜ 0.54.0 None 0 39.4 kB boshen

View full report↗︎

@Boshen Boshen added this pull request to the merge queue Mar 4, 2025
Merged via the queue into main with commit d9e2677 Mar 4, 2025
25 checks passed
@Boshen Boshen deleted the next-oxc branch March 4, 2025 04:05
@sapphi-red sapphi-red mentioned this pull request May 13, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@IWANABETHATGUY IWANABETHATGUY IWANABETHATGUY approved these changes

@sapphi-red sapphi-red sapphi-red approved these changes

Assignees

No one assigned

Labels

needs-triage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Boshen @IWANABETHATGUY @sapphi-red