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

fix(vue-app): reuse page component on watchQuery #5757

Merged
merged 3 commits into from
May 19, 2019
Merged

fix(vue-app): reuse page component on watchQuery #5757

merged 3 commits into from
May 19, 2019

Conversation

hatashiro
Copy link

@hatashiro hatashiro commented May 17, 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

Related issue and pull request:

I've submitted the pull request #5516 and it's about making watchQuery considered on creating a route key. However, by the change, page components are no more reused but recreated for every query route specified in watchQuery (#5738). The pull request was actually a breaking change.

The main goal of the original pull request was to make scroll restoration work with query route. Disabling component reuse is unexpected side-effect. Thus I'd like to revert the original change, with still enabling scroll restoration in a different way. So, here is this pull request.

This pull request restores the original routerViewKey behaviour ($route.path) so that page components are reused. Instead, I added $emit('triggerScroll') in fixPrepatch, which is called only when navigating on a different route but the same component is used according to the function's comment. I tested the change out on example pages and it worked well. The original test cases on scroll behaviour pass too.

Because of this change the documentation about routerViewKey should be updated (watchQuery no more affects the key). I'll update it and upload a pull request on docs after this pull request is reviewed.

Thanks in advance!

Checklist:

@hatashiro hatashiro changed the title Reuse page component on watchQuery fix(vue-app): reuse page component on watchQuery May 17, 2019
@codecov-io
Copy link

Codecov Report

Merging #5757 into dev will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              dev   #5757      +/-   ##
=========================================
- Coverage   95.64%   95.6%   -0.04%     
=========================================
  Files          82      82              
  Lines        2662    2662              
  Branches      683     683              
=========================================
- Hits         2546    2545       -1     
- Misses         97      98       +1     
  Partials       19      19
Impacted Files Coverage Δ
packages/vue-renderer/src/renderer.js 93.49% <0%> (-0.82%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d5a32b...2bb5ce1. Read the comment docs.

@codecov-io
Copy link

codecov-io commented May 17, 2019

Codecov Report

Merging #5757 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##             dev   #5757   +/-   ##
=====================================
  Coverage   95.6%   95.6%           
=====================================
  Files         82      82           
  Lines       2662    2662           
  Branches     683     683           
=====================================
  Hits        2545    2545           
  Misses        98      98           
  Partials      19      19
Flag Coverage Δ
#e2e 100% <ø> (ø) ⬆️
#fixtures 50.67% <ø> (ø) ⬆️
#unit 92.56% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dbb9f03...1b45a95. Read the comment docs.

clarkdo
clarkdo previously approved these changes May 17, 2019
@clarkdo
Copy link
Member

clarkdo commented May 17, 2019

Documentation also needs to be reverted 😄

@hatashiro
Copy link
Author

I've submitted a pull request on docs too.

@pi0 pi0 requested a review from clarkdo May 19, 2019 18:52
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.

watchQuery on page re-creates page component
5 participants