fix(hmr): accept hot updates for modules above page templates #29752
Conversation
gatsbot
bot
added
the
status: triage needed
pieh
added
topic: core
| // this is a bit hacky - exclude expect string or regexp or array of those | ||
| // so this is tricking ReactRefreshWebpackPlugin that we provide RegExp | ||
| exclude: { | ||
| test: modulePath => { | ||
| // when it's not coming from node_modules we treat it as a source file. | ||
| if (!vendorRegex.test(modulePath)) { | ||
| return false | ||
| } | ||
|
|
||
| // If the module uses Gatsby as a dependency | ||
| // we want to treat it as src because of shadowing | ||
| return !modulesThatUseGatsby.some(module => | ||
| modulePath.includes(module.path) | ||
| ) | ||
| }, | ||
| } as RegExp, |
There was a problem hiding this comment.
This feels very hacky and if we would go with this - I'd probably suggest pinning @pmmmwh/react-refresh-webpack-plugin to exact version as this only implement test method of RegExp and this might break spectacularly if other methods would be used in future versions.
Ideally we could ask for support of function for exclude, but this is also weird because that webpack plugin use helpers provided by webpack ( https://github.com/webpack/webpack/blob/04a7d052b5c04628257ec23244f234622c7f394c/lib/ModuleFilenameHelpers.js#L230-L257 ) which just don't handle functions (weird part to me is that rules do, so why would that helper not support is a bit baffling)
| function RootWrappedWithOverlayAndProvider() { | ||
| return ( | ||
| <FastRefreshOverlay> | ||
| <StaticQueryStore>{rootWrappedWithWrapRootElement}</StaticQueryStore> | ||
| </FastRefreshOverlay> | ||
| ) | ||
| } | ||
|
|
||
| export default RootWrappedWithOverlayAndProvider |
There was a problem hiding this comment.
Name our Root Component :)
| const preferDefault = m => (m && m.default) || m | ||
| const Root = preferDefault(require(`./root`)) |
There was a problem hiding this comment.
This was causing some weird module dependency chains and it wasn't exactly playing nice with HMR - hot updates for gatsby-browser modules were declined even tho I was accepting them on line 23 (for ./api-runner-browser).
I'm not exactly sure why it was done this way (delayed commonjs import) :/
pieh
force-pushed
the
v3/accept-hot-update-above-page-templates
branch
from
ec4b6d2
to
7e1a0f4
Compare
pieh
force-pushed
the
v3/accept-hot-update-above-page-templates
branch
from
81b6e7d
to
168a79c
Compare
|
Danger run resulted in 1 fail and 1 markdown; to find out more, see the checks page. Generated by 🚫 dangerJS |
| export const wrapRootElement = ({ element }) => ( | ||
| <WrapRootElement element={element} /> | ||
| ) |
There was a problem hiding this comment.
This is actually important - whatever we pass to wrapRootElement (or wrapPageElement) is NOT treated as component. We just straight call this API (and not use React.createElement) - that's because this is gatsby-browser API hook and we also pass pluginOptions as second parameter. Because of above this is actually not represented as React Component in react tree and react-refresh won't actually apply HMR to it (even tho webpack plugin would treat Wrapper as react component and register it as such). On top of problems there are multiple exports in gatsby-browser and that is also no-no for react-refresh.
Above is also reason why we can't use hooks in there etc
This might need some additional notes in https://www.gatsbyjs.com/docs/reference/config-files/gatsby-browser/#wrapPageElement (and maybe migration guide?)
sidharthachatterjee
deleted the
v3/accept-hot-update-above-page-templates
branch
* fix(hmr): accept hot updates for modules above page templates * actually use fast-refresh for gatsby-browser and shadowed theme-ui config * exclude actually need to be instance of RegExp * tmp * more tmp * init navigation after loader is set * cleanup tmp code * init navigation after resources for current page are loaded * remove commented out code * revert devtool change (cherry picked from commit 55778eb)
#29835) * fix(hmr): accept hot updates for modules above page templates * actually use fast-refresh for gatsby-browser and shadowed theme-ui config * exclude actually need to be instance of RegExp * tmp * more tmp * init navigation after loader is set * cleanup tmp code * init navigation after resources for current page are loaded * remove commented out code * revert devtool change (cherry picked from commit 55778eb) Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
Description
After dropping
react-hot-loaderwe need to accept hot updates ourselves. We do so today for page templates but not above those. Anything imported bygatsby-browserfiles is above page templates in webpack's hierarchy and so those are not handled.TODO:
Related Issues
system-ui/theme-ui#1440 (not in our repo, but pretty much our issue)