Skip to content

fix: correct the behavior when multiple transform filter option are specified#4059

Merged
IWANABETHATGUY merged 1 commit intomainfrom
04-08-fix_multiple_transform_filter_option
Apr 8, 2025
Merged

fix: correct the behavior when multiple transform filter option are specified#4059
IWANABETHATGUY merged 1 commit intomainfrom
04-08-fix_multiple_transform_filter_option

Conversation

@IWANABETHATGUY
Copy link
Member

@IWANABETHATGUY IWANABETHATGUY commented Apr 8, 2025 •
edited
Loading

Description

  1. If user specified multiple transform_filter option at the same time,
    now return false if match any exclude, return true only all include are matched

@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review April 8, 2025 12:47
@IWANABETHATGUY Graphite App
Copy link
Member Author

IWANABETHATGUY commented Apr 8, 2025 •
edited
Loading

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

@netlify
Copy link

netlify bot commented Apr 8, 2025

✅ Deploy Preview for rolldown-rs canceled.

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

@IWANABETHATGUY IWANABETHATGUY changed the title fix: multiple transform filter option fix: behavior when multiple transform filter option are specified Apr 8, 2025
@IWANABETHATGUY IWANABETHATGUY mentioned this pull request Apr 8, 2025
Merged
hyf0
hyf0 approved these changes Apr 8, 2025
View reviewed changes
Copy link
Member

@hyf0 hyf0 left a comment

Choose a reason for hiding this comment

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

Your title lacks a verb :)

@IWANABETHATGUY
Copy link
Member Author

IWANABETHATGUY commented Apr 8, 2025 •
edited
Loading

Your title lacks a verb :)

Yeah, the verb is fix haha, I added it.

@IWANABETHATGUY IWANABETHATGUY changed the title fix: behavior when multiple transform filter option are specified fix: correct the behavior when multiple transform filter option are specified Apr 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2025

Benchmarks Rust

  • target: main(f4c5ff2)
  • pr: 04-08-fix_multiple_transform_filter_option(e1567b1)
group                                                               pr                                     target
-----                                                               --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol                     1.06     77.0±9.94ms        ? ?/sec    1.00     72.8±1.77ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify              1.00     92.2±1.12ms        ? ?/sec    1.01     93.0±1.21ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify-sourcemap    1.00    104.5±0.89ms        ? ?/sec    1.03    107.6±2.05ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap           1.00     80.4±1.17ms        ? ?/sec    1.01     81.4±0.79ms        ? ?/sec
bundle/bundle@rome-ts                                               1.00    119.2±1.67ms        ? ?/sec    1.01    120.9±1.09ms        ? ?/sec
bundle/bundle@rome-ts-minify                                        1.04    205.5±8.56ms        ? ?/sec    1.00    197.5±4.98ms        ? ?/sec
bundle/bundle@rome-ts-minify-sourcemap                              1.01    236.4±2.58ms        ? ?/sec    1.00    233.6±2.96ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                                     1.00    132.8±2.33ms        ? ?/sec    1.00    132.6±1.85ms        ? ?/sec
bundle/bundle@threejs                                               1.00     41.9±2.21ms        ? ?/sec    1.03     43.0±1.51ms        ? ?/sec
bundle/bundle@threejs-minify                                        1.00     84.9±1.08ms        ? ?/sec    1.00     84.9±1.45ms        ? ?/sec
bundle/bundle@threejs-minify-sourcemap                              1.00    105.4±1.27ms        ? ?/sec    1.00    105.6±0.57ms        ? ?/sec
bundle/bundle@threejs-sourcemap                                     1.00     47.8±0.74ms        ? ?/sec    1.01     48.3±0.68ms        ? ?/sec
bundle/bundle@threejs10x                                            1.02    431.0±4.29ms        ? ?/sec    1.00    423.4±3.18ms        ? ?/sec
bundle/bundle@threejs10x-minify                                     1.00   1040.6±6.54ms        ? ?/sec    1.02   1058.0±8.95ms        ? ?/sec
bundle/bundle@threejs10x-minify-sourcemap                           1.00   1223.5±8.50ms        ? ?/sec    1.01  1234.4±11.17ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                                  1.00    490.6±3.01ms        ? ?/sec    1.01    494.2±2.85ms        ? ?/sec
remapping/remapping                                                 1.05     27.1±0.38ms        ? ?/sec    1.00     25.9±0.75ms        ? ?/sec
remapping/render-chunk-remapping                                    1.00     63.9±0.30ms        ? ?/sec    1.08     69.2±5.56ms        ? ?/sec
scan/scan@rome-ts                                                   1.10    106.4±2.44ms        ? ?/sec    1.00     96.7±1.35ms        ? ?/sec
scan/scan@threejs                                                   1.00     32.0±0.51ms        ? ?/sec    1.02     32.8±1.74ms        ? ?/sec
scan/scan@threejs10x                                                1.01    326.8±7.61ms        ? ?/sec    1.00    323.7±4.46ms        ? ?/sec

sapphi-red
sapphi-red approved these changes Apr 8, 2025
View reviewed changes
Copy link
Member

@sapphi-red sapphi-red left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks!

I think we also need to update the docs.

@IWANABETHATGUY IWANABETHATGUY added this pull request to the merge queue Apr 8, 2025
Merged via the queue into main with commit 92f7cc6 Apr 8, 2025
35 checks passed
@IWANABETHATGUY IWANABETHATGUY deleted the 04-08-fix_multiple_transform_filter_option branch April 8, 2025 17:54
This was referenced Apr 9, 2025
Merged
Merged
Merged
github-merge-queue bot pushed a commit that referenced this pull request Apr 9, 2025
@sapphi-red @IWANABETHATGUY
docs: correct description of multiple filter property case (#4065)
9140bc7 
<!-- Thank you for contributing! -->

### Description

Follow up to #4059

<!-- Please insert your description here and provide especially info
about the "what" this PR is solving -->

---------

Co-authored-by: IWANABETHATGUY <iwanabethatguy@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

@hyf0 hyf0 hyf0 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

@IWANABETHATGUY @sapphi-red @hyf0