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

feat(vue-app): trigger watchParam when param is changed in same route #6244

Merged
merged 13 commits into from
Mar 13, 2020

Conversation

clarkdo
Copy link
Member

@clarkdo clarkdo commented Aug 18, 2019

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Try to resolve #6235 , need more discussion.

Currently when dynamic path is changed (it means same route: like /test1 -> /test2 under /:test), we will always call asyncData instead of reusing, I'm not sure it this is in purpose @atinux @pi0 .

After the pr, asyncData will be called in client side when:

  • route is changed (checking by from.name !=== to.name)
  • route is not change, but param is changed and watchQuery is true
  • route is not change, but query is changed and watchQuery is true

If current behaviour is correct and by design, how do you think adding a watchParam and make default value as true which means by default always calling asyncData when changing slug path.

I've changed this pr for adding watchParam

Behaviour after this pr:

  • param is changed with same component, call asyncData
    • if only child param changed, call asyncData of child
    • if watchParam is false, not call asyncData of child
  • query is changed with same component, not call asyncData
  • both param and query are changed with same component, call asyncData

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (PR: #)
  • I have added tests to cover my changes (if not applicable, please state why)
  • All new and existing tests are passing.

@clarkdo clarkdo changed the title fix: trigger watchQuery when param or query are changed in same route feat: trigger watchParam when param is changed in same route Aug 18, 2019
@atinux
Copy link
Member

atinux commented Aug 19, 2019

It was on purpose to call asyncData when the param changes but the same Component is used (actually it's a feature I missed in vue-router and did not want to people to use watch: $route).

I like the idea of introducing watchParam: false to disable it (even if I don't see a real use case)

@clarkdo
Copy link
Member Author

clarkdo commented Aug 19, 2019

@atinux Shared parent compoent may be a use case ? #6235

@clarkdo
Copy link
Member Author

clarkdo commented Aug 19, 2019

@atinux According to children.patch.browser.test, it seemed that asyncData is not called when param is changed.

Current behaviour is:

  • param is changed with same component, not call asyncData
  • query is changed with same component, not call asyncData
  • both param and query are changed with same component, call asyncData

I don't think it's a correct behaviour, with the current pr, behaviour is:

  • param is changed with same component, call asyncData
  • query is changed with same component, not call asyncData
  • both param and query are changed with same component, call asyncData

What do you think ?

@atinux
Copy link
Member

atinux commented Aug 21, 2019

I agree with the behaviour you describe, but we have to keep in mind about the Parent -> Child calls too,

  • param is changed with same component, call asyncData
    • if only child param changed, call asyncData of child
  • query is changed with same component, not call asyncData (expect if use watchQuery)
  • both param and query are changed with same component, call asyncData

@stale stale bot added the stale label Sep 24, 2019
@pi0 pi0 unassigned clarkdo Sep 26, 2019
@stale stale bot removed the stale label Sep 26, 2019
@nuxt nuxt deleted a comment from stale bot Sep 26, 2019
@stale stale bot added the stale label Oct 22, 2019
@nuxt nuxt deleted a comment from stale bot Oct 23, 2019
@stale stale bot removed the stale label Oct 23, 2019
@galvez
Copy link

galvez commented Nov 24, 2019

@atinux @clarkdo is there anything left to do here other than update tests?

@pi0 pi0 force-pushed the dev branch 2 times, most recently from 007c785 to a84f31d Compare January 21, 2020 12:42
@msonowal
Copy link
Contributor

msonowal commented Feb 3, 2020

@atinux
Why this is not merged till now as its a must have like watchQuery

pi0
pi0 previously approved these changes Mar 10, 2020
atinux
atinux previously approved these changes Mar 10, 2020
Copy link
Member

@atinux atinux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve but missing the docs PR :)

@pi0 pi0 dismissed stale reviews from atinux, galvez, and themself via 5aee0a8 March 10, 2020 14:48
@pi0 pi0 changed the title feat: trigger watchParam when param is changed in same route feat(vue-app): trigger watchParam when param is changed in same route Mar 10, 2020
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 this pull request may close these issues.

Strange behaviour of nested pages
7 participants