Add call to fetchSiteData when reloadClientData is called in runDevserver.js

[krishan711]

Description

Fix for #1093

The suggestion in the issue above was to update the latestState variable directly, but as mentioned this doesn't work. I believe this is primarily because the routes are set already and before is not called when the webpack dev server is invalidated.

Testing locally this works, just wanted a second opinion before i look into adding tests as it may be overly heavy-handed.

Changes/Tasks

• Updated to call buildDevRoutes
• (TODO): add tests if this is agreed to be a viable solution

Motivation and Context

comes from #1093. In summary, if you try to call reloadClientData in static.config.js the reload happens but the data isn't actually re-retrieved.

Types of changes

• Refactoring/add tests (refactoring or adding test which isn't a fix or add a feature)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• I have updated the documentation accordingly
• I have updated the CHANGELOG with a summary of my changes
• My changes have tests around them

[SleeplessByte]

We can't just merge this; it definitely needs a flag. routes can take minutes to build depending on your setup. You don't want a Fast Refresh / Hot Reload to take minutes. However, whilst this will work, it's probably not the optimal solution.

Instead, what the author of #1093 wants is that the siteData is cleared (and reloaded), instead of just the route data. The route data is composed of the routes.data + site.data.

[krishan711]

@SleeplessByte Sure, makes sense. How would you suggest going about it since the current code doesn't refresh the site data? Would it make sense to split out a function that builds the site data route ('/__react-static__/siteData') so i can call that function individually and not affect the other routes?

[SleeplessByte]

That would not be a bad idea. How would you go about to merge that data back into the routeData? As in, what's the idea there to update the routeData as the siteData changes?

[krishan711]

im not totally sure what you mean here, probably because I haven't used routeData and siteData together. why would routeData need to be reloaded if the siteData changes? or where should I look to understand this? I checked https://github.com/react-static/react-static/blob/master/packages/react-static/src/static/getRouteData.js but there is no reference to the siteData. is it because the context has state passed to getData has the siteData in it and people may have code dependent on the siteData?

[krishan711]

actually maybe i dont understand the original comment anymore - it doesnt look like routes is rebuilt in this function from what i can see? it's built in start.js and then passed in via the state and not ever updated from what i can tell.

[SleeplessByte]

Sorry, I was confusing you 😓 😅 . I wasn't talking about siteData but sharedData, as you can see here:

react-static/packages/react-static/src/browser/utils/index.js

Lines 230 to 235 in 611aa82

	export function getFullRouteData(routeInfo) {
	return {
	...(routeInfo.sharedData ? routeInfo.sharedData : {}),
	...routeInfo.data,
	}
	}

Anyway.

@SleeplessByte Sure, makes sense. How would you suggest going about it since the current code doesn't refresh the site data? Would it make sense to split out a function that builds the site data route ('/react-static/siteData') so i can call that function individually and not affect the other routes?

Yes, this works perfectly. I just checked and that's how we have it set up for some of our local forks!

[krishan711]

Ah awesome, I'll try this out this evening and post an update then. Thanks 👍

[krishan711]

Hm, I updated this but actually this doesn't seem to work anymore and I can't see immediately why. Will dig deeper.

[krishan711]

Okay, im no longer sure why things weren't working before. This seems to work for me now, contrary to what the open issue says. @SleeplessByte is this good or am i missing something here?

[krishan711]

Hm, in the issue they only tried to change the code directly in the node_modules folder. This didnt work for me. When i edited the source and published to my own registry and used that instead of react-static everything works as expected. You can try it yourself with "@kibalabs/react-static": "7.2.3-kiba.2"

[SleeplessByte]

Hah. You mean it's just working, always :P ?

I don't have the capacity right now to try it out, but we can always close this and reopen if you find a circumstance where it doesn't work.

[krishan711]

Oh no sorry I meant it works with the addition of the single line i have added in the PR now

[krishan711]

(it is what was suggested in the open issue)

[SleeplessByte]

It looks fine for me. Tests still pass. I doesn't look breaking.

Can you add this to CHANGELOG.md (same format, bottom of the master list)? We'll merge it for 7.4.0 and if it creates bugs we can revert or revisit.

[krishan711]

done. thanks!

[SleeplessByte]

Thank you for the effort @krishan711