fix: bad module order and missing modules when optimizing side effect free modules

[kosciolek]

What kind of change does this PR introduce?

Fix

Did you add tests for your changes?

A temporary failed case so far

I made this test under a temp name, I'll tidy it up, give it a proper name and move it to a proper location once I figure out what exactly is the issue.

Does this PR introduce a breaking change?

No

What needs to be documented once your changes are merged?

Nothing

---

fixes #7094 (hopefully, since this issue evolved over time and covers multiple problems)

[linux-foundation-easycla[bot]]

• :x: - login: @kosciolek . The commit (be3cb13a1d3cff835961f4e4f3299ba70e4ad207, a15828394e409de9fe41153bca07be3f87d14f26) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[webpack-bot]

For maintainers only:

• [ ] This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
• [ ] This needs to be backported to webpack 4 (issue will be created when merged)

[kosciolek]

@alexander-akait There's further explanation in the test file: https://github.com/webpack/webpack/pull/18302/files#diff-704c7db65db3c626574febb499039bd28ea77e5675747e9924b6e3738cda11a9R13

Also you might find this interesting https://github.com/webpack/webpack/pull/18302/files#diff-d7226814b80e1c9c596f24913bb3c05e1ea96def6c20a1313433f225fa4996d8R1

[alexander-akait]

Thank you, I see the problem, let's try a fix from https://github.com/webpack/webpack/issues/7094#issuecomment-2043666038 and look at our tests

[kosciolek]

I removed the block that handles imports (HarmonyImportSpecifierDependency) in optimizeIncomingConnections. The block that handles exports (HarmonyExportImportedSpecifierDependency) is still there.

14 test cases in Test / basic seem to fail:

● StatsTestCases › should print correct stats for concat-and-sideeffects
● StatsTestCases › should print correct stats for scope-hoisting-multi
● StatsTestCases › should print correct stats for side-effects-issue-7428
● StatsTestCases › should print correct stats for side-effects-optimization
● StatsTestCases › should print correct stats for side-effects-simple-unused
● TestCases › normal › optimize › side-effects-all-chain-unused › exported tests › should not evaluate a chain of modules
● TestCases › normal › optimize › side-effects-immediate-unused › exported tests › should not evaluate an immediate module
● TestCases › normal › optimize › side-effects-root-unused › exported tests › should evaluate all modules
● TestCases › normal › optimize › side-effects-transitive-unused › exported tests › should not evaluate a reexporting transitive module
● TestCases › normal › side-effects › dynamic-reexports › dynamic-reexports should load the compiled tests
● ConfigTestCases › concatenate-modules › side-effects › exported tests › should import side-effect-free modules in deterministic order (usage order)
● ConfigTestCases › side-effects › side-effects-unsorted-modules › exported tests › should not contain side-effect-free modules
● ConfigTestCases › side-effects › type-reexports › exported tests › should skip over module
● ConfigTestCases › side-effects › type-reexports › exported tests › should skip over module

[alexander-akait]

Yeah, I see it, need to investigate it

[tep-pl]

kosciolek, alexander-akait, any updates on this?

[kosciolek]

@alexander-akait Would you look at it, or should I try?

[alexander-akait]

Looks like your solution broke tests, we need to seach better way to fix it