Skip to content
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

serverPrefetch hooks in mixins not called during SSR #2889

Closed
tabjy opened this issue Dec 26, 2020 · 9 comments
Closed

serverPrefetch hooks in mixins not called during SSR #2889

tabjy opened this issue Dec 26, 2020 · 9 comments

Comments

@tabjy
Copy link

tabjy commented Dec 26, 2020

Version

3.0.4

Reproduction link

https://codesandbox.io/s/vue-ssr-bug-o1nbv?file=/src/index.js

Steps to reproduce

const MyComponent = defineComponent({
  name: "MyComponent",
  template: `<div>MyComponent</div>`,
  mixins: [
    {
      serverPrefetch() {
        console.log("mixin defined serverPrefetch call on", this.$options.name);
        return Promise.resolve();
      }
    }
  ]
});

The serverPrefetch hook mixed into MyComponent is not called during SSR.

What is expected?

When refreshing the SSR page, it is expected to see serverPrefetch hooks for both ComponentA (defined explicitly as a local option) and ComponentB (defined via a mixin) invoked by server renderer.

$ node index.js
serving on port 8080
locally defined serverPrefetch call on ComponentA
mixin defined serverPrefetch call on ComponentB

What is actually happening?

Only the locally defined hook for ComponentA is invoked:

$ node index.js
serving on port 8080
locally defined serverPrefetch call on ComponentA

I'm not very familiar with Vue. Sorry if this turns out to be an id10t error.

@tabjy
Copy link
Author

tabjy commented Dec 26, 2020

Took a deeper look in the source. render.ts doesn't really check if there is any serverPrefetch defined in mixins. Is this an intentional design? It seems to be supported in vue 2 though.

@edison1105
Copy link
Member

The logic for processing mixin serverPrefetch should be written here:
https://github.com/vuejs/vue-next/blob/8ac2241b22/packages/runtime-core/src/componentOptions.ts#L438

@tabjy
Copy link
Author

tabjy commented Dec 27, 2020

Hey @edison1105! Thanks for the timely reply. So is it confirmed to be a bug of Vue 3?

I am not sure if writing serverPrefech processing logic where you pointed is a good idea. To my limited knowledge of Vue:

  1. servePrefetch is only related to SSR, namely the server-renderer package. Being a not standard lifecycle, it feels questionable to process servePrefetch in the runtime-core package.

  2. applyOptions deals with post-created lifecycles only, all of which are essentially noops for SSR.

Please correct me. It's very likely that I misunderstood something here and there...

@edison1105
Copy link
Member

edison1105 commented Dec 27, 2020

I'm think the best is to make it consistent with Vue 2.
https://github.com/vuejs/vue/blob/dev/src/core/util/options.js#L172

BTW, your code not handled globalMixins

@tabjy
Copy link
Author

tabjy commented Dec 27, 2020

...make it consistent with Vue 2

Thank you for pointing the direction. I'll take a deeper look!

BTW, your code not handled globalMixins

No, it doesn't, and it doesn't handle mixins with a mixin either. By no means that was a proper fix. 😂

@posva posva changed the title Mixed in serverPrefetch hooks not called during SSR serverPrefetch hooks in mixins not called during SSR Dec 28, 2020
@edison1105
Copy link
Member

@tabjy I think you can submit your PR if it is ready.

@tabjy
Copy link
Author

tabjy commented Dec 29, 2020

@edison1105 I don't know the workflow here. I was under the impression submitting it after someone from @vuejs confirms it to be a bug and assigns me to it, but you're right. I might as well submit it now. Worst case they can just close it if it's a wontfix.

Thank you!

@HcySunYang
Copy link
Member

@tabjy Thanks your PR, and sorry for being late.
The merge logic of the serverPrefect hook cannot be written in applyOptions, because applyOptions is controlled by __FEATURE_OPTIONS_API__, details can be found here feature-flags. I will take a closer look at this issue later.

@HcySunYang
Copy link
Member

After this #3070, I think it's ok to define merge logic in applyOptions

@tabjy tabjy closed this as completed Oct 12, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Oct 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants