-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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
feat(ssr): ssrPrefetch option #9017
Conversation
@yyx990803 I added |
What's the status of merging this PR? I'm in npm version hell because of it and and my project is screaming at me with errors. The bottleneck is getting caused by version mismatches of |
@dylanmcgowan this is not a bug fix so likely unrelated to whatever error you are seeing. |
@dylanmcgowan As Evan said it's not a bug fix around Vue itself, but a feature (as declared in PR). The thing is that this new feature will allow @Akryum to fix issues within |
Related docs changes: https://github.com/vuejs/vue-ssr-docs/pull/214/files |
Pardon the late reply. To reiterate what @kevinmarrec said and to clarify why I commented in the first place, there are open issues with |
if (!Array.isArray(handlers)) handlers = [handlers] | ||
try { | ||
const promises = [] | ||
for (let i = 0, j = handlers.length; i < j; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Akryum pardon me the intrusion -- just thought i'd drop this here -- I see that you're using a pattern for for
loops that I used to employ myself, but, unless you assign the length to an external variable, this idiom actually makes things slower :(
The more environment specific your hooks get the more likely you are to forget fetch data and have to repeat yourself. Let me demonstrate with a profile route component: <template>
<div> {{ user.name }} </div>
</template>
<script>
export default {
ssrPrefetch() {
return this.$store.dispatch('loadUser', this.$route.params.userId)
},
computed: {
user() {
return this.$store.getters.users(this.$route.params.userId)
}
}
}
</script> I'll test loading my route from the server and everything works fine. However, it's easy to forget that you have to add a way to fetch data for the client render context. To be fair @Akryum does a good job of mentioning this in the docs example. Let's fix up the route: <template>
<div v-if='user'> {{ user.name }} </div>
</template>
<script>
export default {
ssrPrefetch() {
return this.$store.dispatch('loadUser', this.$route.params.userId)
},
mounted () {
if(!this.user) {
this.$store.dispatch('loadUser', this.$route.params.userId)
}
},
computed: {
user() {
return this.$store.getters.users(this.$route.params.userId)
}
},
}
</script> Ok getting closer. Now later on someone ends up adding links to this page that link to other user profiles. When clicking on those links, neither the <template>
<div v-if='user'> {{ user.name }} </div>
</template>
<script>
export default {
ssrPrefetch() {
return this.$store.dispatch('loadUser', this.$route.params.userId)
},
watch:{
$route (to, from) {
this.$store.dispatch('loadUser', to.params.userId)
},
},
mounted () {
if(!this.user) {
this.$store.dispatch('loadUser', this.$route.params.userId)
}
},
computed: {
user() {
return this.$store.getters.users(this.$route.params.userId)
}
},
}
</script> As one can see adding one piece of async data gets pretty complicated. What if instead we could do something like this: <template>
<div v-if='user'> {{ user.name }} </div>
</template>
<script>
export default {
routeData(store, to, from) {
return this.$store.dispatch('loadUser', this.$route.params.userId)
},
computed: {
user() {
return this.$store.getters.users(this.$route.params.userId)
}
},
}
</script> This is achievable (won't go into implementation here) and the main advantage being: Whether the route rendering context is server, client, or client update it will always get called so the first time you develop this it just works in all contexts. Bigger Idea
What if you could just drop your data management into those buckets and you never had to worry about the request context (i.e. server, client, client update) I should write a blog post. |
@trainiac |
@Akryum Yes, I see that. It seems especially helpful for a GraphQL UI. Nice work! Hopefully my little blurb might help give others context to what this is trying to solve and some of the decision making that goes into where it's best to make API requests in an SSR app. Might make sense to add the example of using |
Or rather, since this PR is focused on enabling data fetching capabilities for all components, using a watcher on |
Looks very good!!!! if have this, I can do more things like executing |
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
dev
branch for v2.x (or to a previous version branch), not themaster
branchfix #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information:
ssrPrefetch
context.rendered = context => {}