Skip to content

Conversation

@hai-x
Copy link
Member

@hai-x hai-x commented Jan 24, 2026

Summary

What kind of change does this PR introduce?

Fixes part of #20209.

Did you add tests for your changes?

Yes

Does this PR introduce a breaking change?

Theoretically not

If relevant, what needs to be documented once your changes are merged or what have you already documented?

No

@changeset-bot
Copy link

changeset-bot bot commented Jan 24, 2026

🦋 Changeset detected

Latest commit: 288c500

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
webpack Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Jan 24, 2026

This PR is packaged and the instant preview is available (5ecc58d).

Install it locally:

  • npm
npm i -D webpack@https://pkg.pr.new/webpack@5ecc58d
  • yarn
yarn add -D webpack@https://pkg.pr.new/webpack@5ecc58d
  • pnpm
pnpm add -D webpack@https://pkg.pr.new/webpack@5ecc58d

@hai-x hai-x force-pushed the asset-side-effect-free branch from 2bed3ee to c56b046 Compare January 24, 2026 12:37
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 24, 2026

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing asset-side-effect-free (288c500) with main (fe48655)1

Summary

✅ 72 untouched benchmarks

Footnotes

  1. No successful run was found on main (cd4793d) during the generation of this report, so fe48655 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@alexander-akait
Copy link
Member

Looks like we need to fix tests, but code looks good

@hai-x
Copy link
Member Author

hai-x commented Jan 27, 2026

We should consider a better approach for setting factoryMeta. It is assigned at parsing stage right now, it will lead to some issue when working with both disk and memory caching, and cause module.factoryMeta can't be properly restored.

@alexander-akait
Copy link
Member

@hai-x Yeah, we have the same problem with renaming modules, alternative solution - make builtMeta and buildInfo as getters, but will be glad to have a hook to setup it

@hai-x
Copy link
Member Author

hai-x commented Jan 29, 2026

I use createModuleClass here cause it will be called at the factory stage. And it also aligns with our goal of creating asset modules via newly AssetModule(it's still TODO).

@hai-x hai-x marked this pull request as ready for review January 29, 2026 18:16
})
);
}
});
Copy link
Member

Choose a reason for hiding this comment

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

What about to rewrite test to see that unused module was removed, we have __webpack_modules__ to check them in chunk

@hai-x hai-x force-pushed the asset-side-effect-free branch from 3302b9d to 54d2801 Compare February 1, 2026 17:07
@hai-x hai-x force-pushed the asset-side-effect-free branch from 4d2a9c1 to c3b05b6 Compare February 1, 2026 17:14
@hai-x
Copy link
Member Author

hai-x commented Feb 1, 2026

Thanks for review. It such one good idea to use __webpack_modules__.

@alexander-akait alexander-akait merged commit 5ecc58d into main Feb 2, 2026
49 of 52 checks passed
@alexander-akait alexander-akait deleted the asset-side-effect-free branch February 2, 2026 16:30
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