Your org has enabled the Graphite merge queue for merging into main

Add the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

• feat(transformer/async-to-generator): handle this within arrows function #7074
• perf(transformer/arrow-function): store scope_id to avoid going through scopes #7041
• ci(transformer): enable unfinished plugins in benchmark #7040 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Dunqing and the rest of your teammates on Graphite

CodSpeed Performance Report

Merging #7040 will degrade performances by 17.14%

_{Comparing 10-31-ci_transformer_enable_unfinished_plugins_in_benchmark (70e2582) with main (001058a)}

Summary

❌ 4 regressions
✅ 26 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

I agree that it's good to measure all our transforms, even the unfinished ones, in benchmarks.

However, enable_all is a public API (like Babel's "force all transforms" option), so in my opinion it seems a bit odd to offer an "include unfinished plugins" option, as that's a bit like offering a "break all your code" option!

I think it'd be preferable to leave enable_all as it is, and manually enable the unfinished transforms in tasks/benchmark/benches/transformer.rs.

Or, if you want to locate this logic in oxc_transformer without exposing it to the user:

impl EnvOptions {
    pub fn enable_all() -> Self {
        Self::enable_all_impl(false)
    }

    /// For internal use only
    #[doc(hidden)]
    pub fn enable_all_impl(include_unfinished_plugins: bool) -> Self {
        // Same as impl in this PR
    }
}

Benchmarks can use enable_all_impl, but we don't "officially" expose ability to run our shonky half-finished transforms to the user.

However, enable_all is a public API (like Babel's "force all transforms" option), so in my opinion it seems a bit odd to offer an "include unfinished plugins" option, as that's a bit like offering a "break all your code" option!

I think it'd be preferable to leave enable_all as it is, and manually enable the unfinished transforms in tasks/benchmark/benches/transformer.rs.

We tend to forget to enable unfinished plugins in the benchmark easily. So I am not a fan of this way

Or, if you want to locate this logic in oxc_transformer without exposing it to the user:

impl EnvOptions {
    pub fn enable_all() -> Self {
        Self::enable_all_impl(false)
    }

    /// For internal use only
    #[doc(hidden)]
    pub fn enable_all_impl(include_unfinished_plugins: bool) -> Self {
        // Same as impl in this PR
    }
}

Benchmarks can use enable_all_impl, but we don't "officially" expose ability to run our shonky half-finished transforms to the user.

IMO, enable_all is also an internal-only API. So we can add

/// For internal use only
#[doc(hidden)]

to enable_all method. Tell me what's your thought about this

@Boshen Is enable_all meant to be a public method or not? I thought maybe it is (like Babel's "force all transforms" option).

@Boshen Is enable_all meant to be a public method or not? I thought maybe it is (like Babel's "force all transforms" option).

This is not a public API, but I need this in monitor-oxc.

OK great. @Dunqing I had misunderstood. I had thought enable_all was a public API.

Since it's not, this PR with this added to enable_all (as you suggested), seems good to me:

/// For internal use only
#[doc(hidden)]

Sorry I wasted time!

Merge activity

• Nov 2, 2:17 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Nov 2, 2:18 AM EDT: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..
• Nov 2, 2:18 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Nov 2, 2:26 AM EDT: A user added this pull request to the Graphite merge queue.
• Nov 2, 2:27 AM EDT: The Graphite merge queue couldn't merge this PR because it had conflicts with the trunk branch.
• Nov 2, 11:16 AM EDT: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Nov 2, 11:17 AM EDT: A user added this pull request to the Graphite merge queue.
• Nov 2, 11:22 AM EDT: A user merged this pull request with the Graphite merge queue.