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

Incorrect handling of metadata with navigation #32

Closed
jazoom opened this issue Nov 26, 2016 · 5 comments · Fixed by #81
Closed

Incorrect handling of metadata with navigation #32

jazoom opened this issue Nov 26, 2016 · 5 comments · Fixed by #81
Labels

Comments

@jazoom
Copy link
Contributor

jazoom commented Nov 26, 2016

Say the app.vue parent component at route / has this property:

metaInfo () {
    return {
      title: 'parent title'
    }
}

and a child component at route /child has this property:

 metaInfo() {
    return {
      title: `${this.name}`
    }
 },

When navigating to the child, the title changes correctly. Then when going back, the title stays as the child's title (it should go back to parent title). Then when navigating to the same child component (a different instance), the title flicks to 'undefined' before changing to the new this.name.

What I would expect is that going back from /child to / would change the title back to what it was before the child overrode it. Then I would expect that if this.name is not ready yet when metaInfo is initially called it will leave the current title, rather than changing it to undefined, thus causing the flicker.

@zspecza
Copy link
Contributor

zspecza commented Nov 30, 2016

Hi @jazoom - sorry for the late response. Unfortunately I wasn't able to iterate as fast as I planned on this project and have found myself at a loss for free time while I setup my new business entity, but as soon as I've got some more time on my hands I will be working actively on this project again - should be ETA 1-2 weeks from now.

I did get some free time today to reproduce your issue, but haven't been able to. I do think however that after examining my implementation, there is room for aggressive checking for race conditions that might mitigate this issue.

If you could try to reproduce the problem in a small example repo I could look at, that might help tremendously - if it isn't too much of a bother 😌

@jazoom
Copy link
Contributor Author

jazoom commented Dec 12, 2016

Hi, @declandewet, sorry it has taken a while. I've set up this project for you:

https://github.com/jazoom/demonstration

It is the same sort of setup that my app has, but very simplified. It perfectly demonstrates the issue. All you have to do is click one button then watch the title on the browser tab. Then press the browser's back button. Then click the other button.

@jazoom
Copy link
Contributor Author

jazoom commented Dec 12, 2016

It uses vbuild, so to set it up you just need to yarn or npm install then npm run dev.

@zspecza
Copy link
Contributor

zspecza commented Dec 13, 2016

Hi @jazoom - I see the issue(s). I'll have a look at fixing them as soon as I can. I think the failure to update the old title comes from your use of router.push, as this bug does not occur when navigating via router-link - still a bug though, so thanks for reporting.

As for undefined - I can do a null check, but would have to maintain a representation of old state which would complicate things a bit and would require some time to implement. In the meantime you can simply set the title to something else - I do this in the vuex-async example: https://github.com/declandewet/vue-meta/blob/master/examples/vuex-async/views/Post.vue#L29

@zspecza zspecza added the bug label Dec 13, 2016
@jazoom
Copy link
Contributor Author

jazoom commented Dec 13, 2016

Isn't it possible to just not change the title (in a setter) if the value provided is falsy?

In that example I provided to you I used router.push(), but actually in my app I use a <router-link> for that purpose, and the bug is still there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants