Unwrap "double errors" to limit cost of Error ctor

[markjm]

Bug report

Related to #13532

What is the current behavior?
As discussed in the previous issue, new-ing up Errors has a high cost. This is especially relevant when using a bunch of type-only imports in typescript. Each of these issues causes a HarmonyLinkingError, which is then immediately wrapped in a ModuleDependencyWarning. This double-wrapping causes additional overhead due to both of them extending Error and paying the price of that.

What is the expected behavior?
My suggestion is to cut out the usage ModuleDependency{Warning,Error} and directly use the errors returned from getWarnings. I have looked through the codebase and dont see anywhere that specifically cares about ModuleDependencyXXX error type over other types of WebpackErrors, so this seems like a safe change to make.

See cpu trace images below:

Before

After

In our very large typescript-based project webpack build, this change saves ~200ms per recompile. Attaching changes.

Other relevant information:
webpack version: 5.55.1
Node.js version: 16.9.0
Operating System: windows
Additional tools: -

[markjm]

@sokra appreciate your recent perf investigations, so trying to help out where I can.

[sokra]

First, have you considered fixing this typescript problem by using export type instead?

Otherwise I would rather look into not generating these warnings in first place:

Either we add a flag to disable these type of warning, e. g. allowing { test: /\.ts$/, parser: { ignoreReexportLinkingErrors: true } }

Or we add a flag to report these warnings only once, e. g. warningsForUnaffectedModules: false.
This would only show errors and warnings for modules that have been affected by the current change. This means warnings that have once been shown to the users are not shown again.

[markjm]

Is it specifically the export type that matters? or also then import type? We have looked at using import type, but the ergonomics are a little tough in terms of making changes (assuming we lint require using import type). You end up with possibly 2 imports to the same location in the same file - one for types and one for non-types, and then as code changes and moves around its just some extra churn of changing imports across the project. I am not totally against it, but dont think we will do that immediately.

I also agree that it is most valuable to add a flag to disable these, but I dont think this needs to be exclusive with the changes presented in my PR above. I think ensuring perf is optimal (1) for those that dont want to enable that new flag or (2) for any other errors & warnings that may be output from getWarnings is still valuable.

Do you have any concerns accepting these changes in addition to my looking into adding this flag?

[sokra]

Afaik that's only related to reexporting so export ... from. In isolated modules mode typescript doesn't know if the reexported thing is a type or a value and (sometimes incorrectly) assumes it's a value.