Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gatsby): Persist manifest on cached builds #36693

Merged
merged 3 commits into from Sep 28, 2022

Conversation

tyhopp
Copy link
Contributor

@tyhopp tyhopp commented Sep 26, 2022

Description

In our current implementation the client module manifest is wiped on subsequent builds, breaking hydration.

This happens because we run PartialHydrationPlugin during the build-javascript stage, and on subsequent builds webpack doesn't revisit untouched files and uses the cache instead.

This change restores the previous manifest when PartialHydrationPlugin runs and merges it with any new client modules found.

React has a similar implementation looking for .client.js files (the old API) in a beforeCompile hook to iterate over later - https://github.com/facebook/react/blob/3f70e68cea8d2ed0f53d35420105ae20e22ce428/packages/react-server-dom-webpack/src/ReactFlightWebpackPlugin.js#L110-L132. We could do something similar, but it would be more expensive with the new API because we'd need to check every file's contents for client export on every run.

Instead our plugin runs alongside JavaScript bundling, relies on webpack to detect touched files, and adds new client modules to the manifest as needed. This won't handle deletion, but deleted client modules don't need to be referenced from the manifest at runtime anyway since they're not loaded.

The way I see it this change fixes the subsequent builds problem for now, and if we see issues in the future with this approach we can re-evaluate using an approach like React's (and test the performance impact).

Documentation

N/A

Related Issues

[sc-55768]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Sep 26, 2022
@tyhopp tyhopp added topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Sep 26, 2022
@tyhopp tyhopp marked this pull request as draft September 26, 2022 09:58
@tyhopp tyhopp added this to the Gatsby 5 milestone Sep 26, 2022
@tyhopp tyhopp marked this pull request as ready for review September 28, 2022 03:38
@tyhopp tyhopp added the topic: webpack/babel Webpack or babel label Sep 28, 2022
@tyhopp tyhopp merged commit 45a105b into master Sep 28, 2022
@tyhopp tyhopp deleted the fix/partial-hydration-manifest-cache branch September 28, 2022 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) topic: webpack/babel Webpack or babel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants