Small dead code removal #10904
Small dead code removal #10904
Conversation
codyrancher
force-pushed
the
small-dead-code-removal
branch
from
b38a716 to
c7824dd
Compare
codyrancher
force-pushed
the
small-dead-code-removal
branch
from
c7824dd to
545dddd
Compare
| @@ -98,81 +89,6 @@ const errorHandler = Vue.config.errorHandler || console.error; // eslint-disable | |||
| // Create and mount App | |||
| createApp(nuxt.publicRuntimeConfig).then(mountApp).catch(errorHandler); // eslint-disable-line no-undef | |||
|
|
|||
| async function loadAsyncComponents(to, from, next) { | |||
There was a problem hiding this comment.
We don't use asyncComponents in the app
https://v2.vuejs.org/v2/guide/components-dynamic-async#Async-Components
| } | ||
|
|
||
| // Get matched components | ||
| function resolveComponents(route) { |
There was a problem hiding this comment.
After removing other code this was no longer used.
| if (to === from) { | ||
| _lastPaths = []; | ||
| } else { | ||
| const fromMatches = []; | ||
|
|
||
| _lastPaths = getMatchedComponents(from, fromMatches).map((Component, i) => { | ||
| return compile(from.matched[fromMatches[i]].path)(from.params); | ||
| }); | ||
| } |
There was a problem hiding this comment.
After removing other code _lastPaths was never read it was only written to.
The risk to removing code in a scenario like this is that a large portion of our code has side effects even in getters. I did look at getMatchedComponents and compile and neither appear to have side effects.
| Components.forEach((Component) => { | ||
| if (Component._Ctor && Component._Ctor.options) { | ||
| Component.options.fetch = Component._Ctor.options.fetch; | ||
| } | ||
| }); |
There was a problem hiding this comment.
@rak-phillip it looks like you were right and we can remove this.
In all the cases I saw when invoking this the method of Component.options.fetch was equal to Component._Ctor.options.fetch so this assignment didn't do anything.
I also verified that HMR was working when modifying code in Project Namespaces.
| let instances; | ||
|
|
||
| // Call fetch hooks on components matched by the route. | ||
| await Promise.all(Components.map((Component, i) => { |
There was a problem hiding this comment.
Ultimately this could be removed because we'd exit early from this before doing any mutations. Our fetch method is still called which is obvious by our e2e tests still passing.
| @@ -432,52 +265,6 @@ function checkForErrors(app) { | |||
| } | |||
| } | |||
|
|
|||
| // When navigating on a different route but the same component is used, Vue.js | |||
| // Will not update the instance data, so we have to update $data ourselves | |||
| function fixPrepatch(to, ___) { | |||
There was a problem hiding this comment.
We would early exit because _dataRefresh was never true (Because we'd always exit early from the // Call fetch hooks on components matched by the route. section).
This was changing behavior from standard vue and I verified that data was still loading by going to two child routes which shared the same components. More specifically, two list pages which hadn't overridden the default (a few random ones under more resources).
| } | ||
|
|
||
| function nuxtReady(_app) { | ||
| window.onNuxtReadyCbs.forEach((cb) => { |
There was a problem hiding this comment.
onNuxtReadyCbs wenre't being added because there weren't any invocations of onNuxtReady. Without onNuxtReadyCbs this invocation is useless.
| } | ||
| }); | ||
| // Special JSDOM | ||
| if (typeof window._onNuxtLoaded === 'function') { |
There was a problem hiding this comment.
No usages of _onNuxtLoaded found.
| // Initialize error handler | ||
| _app.$loading = {}; // To avoid error while _app.$nuxt does not exist | ||
| if (NUXT.error) { |
There was a problem hiding this comment.
No usages of NUXT.error. It's only scoped to this file.
| Vue.nextTick(() => { | ||
| // Call window.{{globals.readyCallback}} callbacks | ||
| nuxtReady(_app); | ||
| }); |
There was a problem hiding this comment.
Explained why nuxtReady wasn't doing anything in a comment above.
| const Components = await Promise.all(resolveComponents(app.context.route)); | ||
|
|
||
| if (Components.length) { | ||
| _lastPaths = router.currentRoute.matched.map((route) => compile(route.path)(router.currentRoute.params)); | ||
| } |
There was a problem hiding this comment.
_lastPaths was only written to after other changes.
| if (NUXT.serverRendered && isSamePath(NUXT.routePath, _app.context.route.path)) { | ||
| return mount(); | ||
| } |
There was a problem hiding this comment.
All changes in this file stemmed from the changes in client.js removing the final use cases of code that was here.
There was a problem hiding this comment.
Performed a similar round of testing as in #10896. Looks great!
Summary
Removing bits and pieces of dead code I found while looking through client.js prompted by #10896 (comment)
Areas or cases that should be tested
Mostly around HMR data and fetch invocations.
Checklist