Preload data bundles for snappy client transitions #74
Conversation
… copies. Shallow copy was being modified by different client transitions?
| @@ -10,8 +11,14 @@ function makeRequest (method, url) { | |||
| return new Request(method, url, API.cache()); | |||
| } | |||
|
|
|||
| const BUNDLE_CACHE = {}; | |||
There was a problem hiding this comment.
Very nitpicky thought: we currently use "bundle" to talk about the static JS bundles, so maybe its worth using a different term here, or a more specific one like DATA_BUNDLE_CACHE.
As always, feel free to ignore.
There was a problem hiding this comment.
Thanks @aickin, I've pushed a patch that uses DATA_BUNDLE to disambiguate.
Just "bundle" is ambiguous since we also use webpack JS bundles.
Eliminate a duplicate magic string
|
This is an absurdly elegant way to deal with bundling and preloading data. @roblg mentioned the bundling to me the other night and I was blown away. Bravo. One thought, perhaps parallel to this PR: I wonder if it's also worthwhile to write some code that allows the pre-loading of the JavaScript and CSS bundles, which are the other potentially significant drivers of latency. It seems to that this solution will be awesome when going from one page of a particular type to another of the same type (like LDP --> LDP), but less so when switching types. I could imagine it just using the built-in browser |
Ooh, yeah! 🚀 Let's see... For the JS bundle webpack will fetch it for us (and get it all prepped) if we call the loader in the routes object. For CSS, unfortunately, I'm not sure if we can cheaply preload since the URLs are returned from Good idea! |
|
Hmmm... good point. You don't want to call Here's a wacky idea: could you call One downside with this idea vs. your current solution is that you'd need to parse the JSON from the server when it's downloaded, rather than when it's needed, which I really like about the current solution. If that was really a problem in practice, you could conceivably make some easier-to-parse format. |
|
Sounds great. Thanks! And fwiw, this PR lgtm (am I even allowed to say that?). |
|
As an aside, Just check Chromium's bug tracker before forging ahead. |
|
Do you have a citation for when that was true? At least according to this presentation by Ilya Grigorik, at least, prefetch hints "have lowest possible priority". |
| @@ -10,8 +11,14 @@ function makeRequest (method, url) { | |||
| return new Request(method, url, API.cache()); | |||
| } | |||
|
|
|||
| const DATA_BUNDLE_CACHE = {}; | |||
There was a problem hiding this comment.
Still trying to read through this PR, but this looks like a module-level variable? Does it have request-specific stuff in it?
There was a problem hiding this comment.
Oh, I see. Can't preload server-side. Still, this variable as a global gives me the heebie-jeebies. Can we add a comment here to that effect as well? I literally almost had a heart attack....
There was a problem hiding this comment.
Heh. Good call. Pushed a patch that both adds a comment and also just makes DATA_BUNDLE_CACHE undefined on the server.
Want to be extra sure that we don't accidentally cache data across requests on the server.
|
https://code.google.com/p/chromium/issues/detail?id=61476 (Maybe this is a bug in Webkit but not in Blink?) |
|
LGTM 👍 And FWIW, @aickin you have my vote to be allowed to say "lgtm" ;) |
Agreed. Thanks for reviewing! |
We access it in a few places, but we don't want to be able to _write_ to it.
Adds a method to preload data for a URL that might be navigated to via client transition.
The data is then cached for the lifetime of the client controller. If a client transition is made to
urlthen theReactServerAgentcache is pre-populated.A new page lifecycle was added to provide bundles of preloaded data. It just runs
handleRoute, waits for data to resolve, and then spits out the RSA cache as a JSON blob. It's triggered by a query-string parameter.TODO: Invalidation. Probably need a method to blow away the preload cache. Maybe also one to drop data for individual URLs.