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): fix `asyncData` memory leak on client-side #4966

Merged
merged 4 commits into from Feb 8, 2019

Conversation

Projects
None yet
5 participants
@pi0
Copy link
Member

pi0 commented Feb 6, 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

As reported by @hangaoke1 in #4554, asyncData in client side, leaks state of the route when navigating in the browser.

More investigation has shown that this is happening because we chain data with already applied asyncData of each component after each visit.

We previously avoided leak for SSR. But client-side was not handled probably. (aaf924f and b49713a)

Also, a regression test added to prevent this problem in the future.

Resolves #4554

@pi0 pi0 added the WIP label Feb 6, 2019

@pi0 pi0 changed the title test: add failing test for client-side asyncData leak fix(vue-app): fix `asyncData` memory leak in client-side Feb 6, 2019

@pi0 pi0 changed the title fix(vue-app): fix `asyncData` memory leak in client-side fix(vue-app): fix `asyncData` memory leak in client-side (#4554) Feb 6, 2019

@pi0 pi0 removed the WIP label Feb 6, 2019

@pi0 pi0 self-assigned this Feb 6, 2019

@pi0 pi0 requested review from Atinux, clarkdo, manniL and nuxt/core-team Feb 6, 2019

@manniL

This comment has been minimized.

Copy link
Member

manniL commented Feb 6, 2019

@pi0 e2e is failing 🤔

@manniL manniL changed the title fix(vue-app): fix `asyncData` memory leak in client-side (#4554) fix(vue-app): fix `asyncData` memory leak on client-side Feb 6, 2019

@pi0 pi0 force-pushed the fix/spa-leak branch from 7a93fdd to d8fd7c4 Feb 6, 2019

@pi0 pi0 changed the base branch from dev to 2.x Feb 6, 2019

@pi0

This comment has been minimized.

Copy link
Member Author

pi0 commented Feb 6, 2019

@manniL Will check that

@Atinux Atinux self-assigned this Feb 8, 2019

@pi0 pi0 merged commit 4086800 into 2.x Feb 8, 2019

9 checks passed

Semantic Pull Request ready to be squashed
Details
ci/circleci: audit Your tests passed on CircleCI!
Details
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: lint-app Your tests passed on CircleCI!
Details
ci/circleci: setup Your tests passed on CircleCI!
Details
ci/circleci: test-e2e Your tests passed on CircleCI!
Details
ci/circleci: test-types Your tests passed on CircleCI!
Details
ci/circleci: test-unit Your tests passed on CircleCI!
Details
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Feb 8, 2019

Codecov Report

Merging #4966 into 2.x will decrease coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             2.x    #4966      +/-   ##
=========================================
- Coverage   91.4%   91.32%   -0.09%     
=========================================
  Files         71       71              
  Lines       2363     2363              
  Branches     584      584              
=========================================
- Hits        2160     2158       -2     
- Misses       185      187       +2     
  Partials      18       18
Impacted Files Coverage Δ
packages/vue-renderer/src/renderer.js 94.56% <0%> (-1.09%) ⬇️

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 17cc12f...17ec846. Read the comment docs.

@pi0 pi0 deleted the fix/spa-leak branch Feb 8, 2019

@pi0 pi0 referenced this pull request Feb 25, 2019

Closed

chore(release): v2.4.4 #5109

@pi0 pi0 referenced this pull request Mar 14, 2019

Merged

v2.5.0 #5237

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.