fix(gatsby): Move "react-dom-server" out of framework chunk #37508
Conversation
gatsbot
bot
added
the
status: triage needed
LekoArts
added
topic: webpack/babel
| @@ -26,10 +26,16 @@ import { shouldGenerateEngines } from "./engines-helpers" | |||
| import { ROUTES_DIRECTORY } from "../constants" | |||
| import { BabelConfigItemsCacheInvalidatorPlugin } from "./babel-loader" | |||
| import { PartialHydrationPlugin } from "./webpack/plugins/partial-hydration" | |||
| import { satisfiesSemvers } from "./flags" | |||
|
|
||
| const FRAMEWORK_BUNDLES = [`react`, `react-dom`, `scheduler`, `prop-types`] | ||
|
|
||
| // This regex ignores nested copies of framework libraries so they're bundled with their issuer | ||
| const FRAMEWORK_BUNDLES_REGEX = new RegExp( |
There was a problem hiding this comment.
Moving regex creation to top. Better performance than creating the regex over and over again
| `|` | ||
| )})[\\\\/]` | ||
| ), | ||
| test: FRAMEWORK_BUNDLES_REGEX, |
There was a problem hiding this comment.
In develop stage we don't need to change anything, react-dom/server is only relevant for build-javascript stage
| module.rawRequest === `react-dom/server` || | ||
| module.rawRequest.includes(`/react-dom-server`) |
There was a problem hiding this comment.
Should I use rawRequest or request? Opinions?
There was a problem hiding this comment.
@wardpeet do you have an opinion here?
There was a problem hiding this comment.
rawRequest as that's the actual user's request. If you use request, it's appended with loaders etc which will be harder to match
| return false | ||
| } | ||
|
|
||
| return FRAMEWORK_BUNDLES_REGEX.test(module.nameForCondition()) |
There was a problem hiding this comment.
We seem to use module.identifier() in other places but most often on the internet I've seen people use module.nameForCondition()
There was a problem hiding this comment.
it doesn't really matter for this use case. nameForCondition returns the resource without any query parts.
https://github.com/webpack/webpack/blob/293e677b355da0d5bc4ddfc97d2afec114912711/lib/NormalModule.js#L375-L380
There was a problem hiding this comment.
this change brakes gatsby-plugin-preact which assumes that test function is a regex
| return false | ||
| } | ||
|
|
||
| return FRAMEWORK_BUNDLES_REGEX.test(module.nameForCondition()) |
There was a problem hiding this comment.
it doesn't really matter for this use case. nameForCondition returns the resource without any query parts.
https://github.com/webpack/webpack/blob/293e677b355da0d5bc4ddfc97d2afec114912711/lib/NormalModule.js#L375-L380
The framework test was changed in gatsby 5.6 via gatsbyjs/gatsby#37508 Therefor framework.test is no longer of type RegExp and fixFrameworkCache would return early. This resulted in the non-creation of framework-[contenthash].js. Instead the framework was then located in app-[contenthash].js Since I didn't find a good way to get the framework bundles, I just inlined them from the gatsby repos. They were not changed since 2020 (the creation of the framework chunking). So this seems pretty stable. Further this now checks for react-dom/server and does not include it in the framework bundle as done in the main gatsby repos by PR linked above.
Description
When one uses
gatsby-plugin-imagethereact-dom-serveris included into the framework chunk.Before:
After:
Related Issues
[ch60411]
Fixes #37281