Skip to content

Comments

fix(plugin/vite-resolve): external was serialized incorrectly#4270

Merged
hyf0 merged 1 commit intomainfrom
fix/plugin-vite-resolve-external-value
Apr 23, 2025
Merged

fix(plugin/vite-resolve): external was serialized incorrectly#4270
hyf0 merged 1 commit intomainfrom
fix/plugin-vite-resolve-external-value

Conversation

@sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Apr 23, 2025 •
edited
Loading

Description

Moved the type conversion logic to rust side so that BindingResolvedExternal is serialized to string or boolean directly rather than via a temporary object.

The alternative way would be to wrap each hook and run transformResolvedExternal here for callable plugins.

rolldown/packages/rolldown/src/builtin-plugin/utils.ts

Lines 21 to 24 in f386130

wrappedPlugin[key] = function(...args) {
// @ts-expect-error
return callablePlugin[key](...args);
};

@netlify
Copy link

netlify bot commented Apr 23, 2025 •
edited
Loading

✅ Deploy Preview for rolldown-rs ready!

Name Link
🔨 Latest commit 9a36f69
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/6808cf7f96061a00081d2f13
😎 Deploy Preview https://deploy-preview-4270--rolldown-rs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sapphi-red sapphi-red force-pushed the fix/plugin-vite-resolve-external-value branch from 13ff918 to caad3d9 Compare April 23, 2025 11:27
@sapphi-red sapphi-red force-pushed the fix/plugin-vite-resolve-external-value branch from caad3d9 to 9a36f69 Compare April 23, 2025 11:31
@sapphi-red sapphi-red mentioned this pull request Apr 23, 2025
Merged
@sapphi-red sapphi-red marked this pull request as ready for review April 23, 2025 11:39
@github-actions
Copy link
Contributor

github-actions bot commented Apr 23, 2025

Benchmarks Rust

  • target: main(f386130)
  • pr: fix/plugin-vite-resolve-external-value(9a36f69)
group                                                               pr                                     target
-----                                                               --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol                     1.04     75.1±1.83ms        ? ?/sec    1.00     72.3±1.30ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify              1.00     93.9±1.16ms        ? ?/sec    1.00     94.4±1.27ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify-sourcemap    1.01    107.8±1.05ms        ? ?/sec    1.00    107.0±1.39ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap           1.03     84.4±1.60ms        ? ?/sec    1.00     82.2±0.98ms        ? ?/sec
bundle/bundle@rome-ts                                               1.00    123.4±1.27ms        ? ?/sec    1.01    124.2±2.27ms        ? ?/sec
bundle/bundle@rome-ts-minify                                        1.00    202.3±3.04ms        ? ?/sec    1.00    203.2±5.00ms        ? ?/sec
bundle/bundle@rome-ts-minify-sourcemap                              1.01    247.5±4.30ms        ? ?/sec    1.00    245.1±3.74ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                                     1.00    136.9±1.67ms        ? ?/sec    1.00    137.0±3.06ms        ? ?/sec
bundle/bundle@threejs                                               1.00     42.0±1.39ms        ? ?/sec    1.01     42.3±0.70ms        ? ?/sec
bundle/bundle@threejs-minify                                        1.01     86.2±1.11ms        ? ?/sec    1.00     85.5±0.96ms        ? ?/sec
bundle/bundle@threejs-minify-sourcemap                              1.00    101.1±0.96ms        ? ?/sec    1.06    107.3±1.19ms        ? ?/sec
bundle/bundle@threejs-sourcemap                                     1.00     48.2±0.31ms        ? ?/sec    1.00     48.3±0.36ms        ? ?/sec
bundle/bundle@threejs10x                                            1.00    430.3±4.89ms        ? ?/sec    1.00    429.2±3.12ms        ? ?/sec
bundle/bundle@threejs10x-minify                                     1.01   1060.9±4.51ms        ? ?/sec    1.00   1052.7±6.31ms        ? ?/sec
bundle/bundle@threejs10x-minify-sourcemap                           1.00   1240.6±7.08ms        ? ?/sec    1.00  1236.3±10.56ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                                  1.01    505.1±5.50ms        ? ?/sec    1.00    501.9±4.38ms        ? ?/sec
remapping/remapping                                                 1.00     27.0±0.72ms        ? ?/sec    1.00     27.0±0.32ms        ? ?/sec
remapping/render-chunk-remapping                                    1.00     64.3±0.67ms        ? ?/sec    1.05     67.7±4.80ms        ? ?/sec
scan/scan@rome-ts                                                   1.00     95.1±1.65ms        ? ?/sec    1.00     95.0±0.93ms        ? ?/sec
scan/scan@threejs                                                   1.00     31.8±0.41ms        ? ?/sec    1.01     32.1±0.44ms        ? ?/sec
scan/scan@threejs10x                                                1.01    317.4±5.60ms        ? ?/sec    1.00    315.4±3.34ms        ? ?/sec

@hyf0 hyf0 added this pull request to the merge queue Apr 23, 2025
Merged via the queue into main with commit 3a98131 Apr 23, 2025
29 checks passed
@hyf0 hyf0 deleted the fix/plugin-vite-resolve-external-value branch April 23, 2025 12:27
github-merge-queue bot pushed a commit that referenced this pull request Apr 23, 2025
@sapphi-red @autofix-ci
feat(plugin/vite-resolve): port changes after Vite 6 (#4269)
7c1fae3 
<!-- Thank you for contributing! -->

### Description

Ported changes from the following PRs:

- vitejs/vite#18889
- vitejs/vite#19300
- vitejs/vite#18584,
vitejs/vite#19312

and some minor changes.

With this PR and #4270, the tests in Vite repo should now pass with
native resolve plugin.

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

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@sapphi-red sapphi-red mentioned this pull request Jul 8, 2025
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jul 8, 2025
@sapphi-red
refactor: move side effects type bindings logic to rust side (#5203)
f9ab773 
Similar to #4270, but for
`BindingHookSideEffects`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

2 participants

@sapphi-red @hyf0