Skip to content

Commit

Permalink
fix webpack resolver state leakage
Browse files Browse the repository at this point in the history
I found a case where we were accidentally leaking state between WebpackModuleRequests. This is hard to avoid since webpack *really* pushes us toward using mutation on the shared ResolveData object.

I solved it by making our mutations happen only at the precise moment when we're letting a request bubble back out to webpack.

This is expected to fail tests however because this bug was masking another bug. Specifically, we have several test apps that use things like the `page-title` helper. That helper is (1) used with ambiguous syntax, (2) published in a v2 addon. It turns out we cannot resolve the helper due to #1686, and this was only "working" by accident since the failed attempt at resolving the helper got leaked back to webpack and webpack *does* understand package.json exports and would find it.
  • Loading branch information
ef4 committed Dec 6, 2023
1 parent 014abac commit d20c7d0
Showing 1 changed file with 22 additions and 0 deletions.
22 changes: 22 additions & 0 deletions packages/webpack/src/webpack-resolver-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,28 @@ class WebpackModuleRequest implements ModuleRequest {
return 'webpack';
}

// Webpack mostly relies on mutation to adjust requests. We could create a
// whole new ResolveData instead, and that would allow defaultResolving to
// happen, but for the output of that process to actually affect the
// downstream code in Webpack we would still need to mutate the original
// ResolveData with the results (primarily the `createData`). So since we
// cannot avoid the mutation anyway, it seems best to do it earlier rather
// than later, so that everything from here forward is "normal".
//
// Technically a NormalModuleLoader `resolve` hook *can* directly return a
// Module, but that is not how the stock one works, and it would force us to
// copy more of Webpack's default behaviors into the inside of our hook. Like,
// we would need to invoke afterResolve, createModule, createModuleClass, etc,
// just like webpack does if we wanted to produce a Module directly.
//
// So the mutation strategy is much less intrusive, even though it means there
// is the risk of state leakage all over the place.
//
// We mitigate that risk by waiting until the last possible moment to apply
// our desired ModuleRequest fields to the ResolveData. This means that as
// requests evolve through the module-resolver they aren't actually all
// mutating the shared state. Only when a request is allowed to bubble back
// out to webpack does that happen.
toWebpackResolveData(): ExtendedResolveData {
this.originalState.request = this.specifier;
this.originalState.context = dirname(this.fromFile);
Expand Down

0 comments on commit d20c7d0

Please sign in to comment.