Remove async data implementation #10896
Conversation
codyrancher
force-pushed
the
remove-async-data-impl
branch
from
3cdf3ec
to
a9e9cea
Compare
| // Hot reloading | ||
| if (isDev) { | ||
| setTimeout(() => hotReloadAPI(this), 100); | ||
| } |
There was a problem hiding this comment.
As mentioned in the PR summary. I noticed that the HMR code wasn't actually being used so I decided to remove it for both fetch and asyncData.
The video I posted demonstrates that HMR was still working in fetch even without this code.
| @@ -505,108 +478,6 @@ function nuxtReady(_app) { | |||
| } | |||
| } | |||
|
|
|||
| const noopData = () => { | |||
There was a problem hiding this comment.
This entire section is for the HMR of fetch and asyncData
| this.error(); | ||
| const promises = []; | ||
| const next = function(path) { | ||
| this.$loading.finish && this.$loading.finish(); |
There was a problem hiding this comment.
This makes me wonder if $loading can be removed entirely. I didn't experience any regressions while clicking through the app in dev.
There was a problem hiding this comment.
Do you mean for this specific case or in general?
| @@ -72,34 +72,6 @@ export function getChildrenComponentInstancesUsingFetch(vm, instances = []) { | |||
| return instances; | |||
| } | |||
|
|
|||
| export function applyAsyncData(Component, asyncData) { | |||
There was a problem hiding this comment.
All invocations of this were removed.
There was a problem hiding this comment.
LGTM - I played around with different scenarios to see if I could trigger some unexpected behavior (managing extensions, mocking development, provisioning clusters, reloading pages, etc...) and I'm unable to find anything wrong with this.
I'm quite glad to see a lot of this code go 😁
| Components.forEach((Component) => { | ||
| if (Component._Ctor && Component._Ctor.options) { | ||
| Component.options.asyncData = Component._Ctor.options.asyncData; | ||
| Component.options.fetch = Component._Ctor.options.fetch; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Based on your other comment about HMR1, would we be able to drop this (fetch) as well?
Footnotes
There was a problem hiding this comment.
We might be able to but I haven't verified yet. It wasn't as obvious while I was digging. I'll take a look and open a separate PR if we can.
| // Hot reloading | ||
| if (isDev) { | ||
| setTimeout(() => hotReloadAPI(this), 100); | ||
| } |
| this.error(); | ||
| const promises = []; | ||
| const next = function(path) { | ||
| this.$loading.finish && this.$loading.finish(); |
There was a problem hiding this comment.
Do you mean for this specific case or in general?
$loading is actually assigned a reference to a component (our loading component) and then these are invocations on that component. |
Summary
Removing asyncData references and unused hot module reload code
Technical notes summary
While removing references to asyncData I noticed that the HMR code used for calling both asyncData and fetch were not actually being used on code changes. This lead me to deleting the HMR code as well. We can split this PR up into two separate if we deem necessary.
Areas or cases that should be tested
asyncData has already been removed so that area shouldn't be affected. We should verify that HMR still works for usages of fetch.
Screenshot/Video
remove-unused-hmr.mp4
Checklist