Skip to content

Add collectDestructuringAasignmentProperties hook to only collect for specific expression#19811

Merged
alexander-akait merged 2 commits intowebpack:mainfrom
ahabhgk:collect-dest-hook
Aug 17, 2025
Merged

Add collectDestructuringAasignmentProperties hook to only collect for specific expression#19811
alexander-akait merged 2 commits intowebpack:mainfrom
ahabhgk:collect-dest-hook

Conversation

@ahabhgk
Copy link
Contributor

@ahabhgk ahabhgk commented Aug 16, 2025

Add collectDestructuringAasignmentProperties hook to only collect destructuringAssignmentProperties for specific expression.

Before all Expression in ObjectPattern = Expression were collected into destructuringAssignmentProperties, but actually we only need to collect some specific expressions: harmonyImportSpecifier, import.meta, import(), and the key of DefinePlugin options, so this PR add a new hook that identify these expressions and only collect destructuringAssignmentProperties for them.

#18319 (comment) And duo to performance reason that destructuringAssignmentProperties only collect properties for simple cases like { a, b: c } = d, after this we can also add collect the nested properties: { a: { b: {c} } } = d in future PR

What kind of change does this PR introduce?

refactoring, and bugfix for ({ webpack } = await ({ url } = import.meta));

Did you add tests for your changes?

Yes

Does this PR introduce a breaking change?

No

What needs to be documented once your changes are merged?

No

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 16, 2025

CodSpeed Performance Report

Merging #19811 will degrade performances by 95.23%

Comparing ahabhgk:collect-dest-hook (f0c8251) with main (613a5ad)

Summary

⚡ 4 improvements
❌ 3 regressions
✅ 35 untouched benchmarks

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

Benchmarks breakdown

Benchmark BASE HEAD Change
benchmark "cache-filesystem", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 240.9 ms 5,047.5 ms -95.23%
benchmark "devtool-eval-source-map", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 43.9 ms 94.1 ms -53.28%
benchmark "devtool-source-map", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 107.1 ms 83.7 ms +27.91%
benchmark "future-defaults", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 90.5 ms 142.3 ms -36.36%
benchmark "many-chunks-commonjs", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 401.7 ms 114 ms ×3.5
benchmark "many-modules-commonjs", scenario '{"name":"mode-development-rebuild","mode":"development","watch":true}' 113.5 ms 36 ms ×3.2
benchmark "react", scenario '{"name":"mode-development","mode":"development"}' 233.2 ms 184.7 ms +26.25%

@3ru
Copy link
Member

3ru commented Aug 17, 2025

@ahabhgk fyi CI failures on Node.js v24.x are due to a known upstream regression in VM module linking (v24.6+). Please ignore these failures for now :)

@alexander-akait alexander-akait merged commit dc79e95 into webpack:main Aug 17, 2025
35 of 41 checks passed
@ahabhgk ahabhgk deleted the collect-dest-hook branch August 17, 2025 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants