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(component): clean up memory leak after loading async component completes (fix #8740) #8755

Merged
merged 3 commits into from
Nov 30, 2018

Conversation

mattlavallee
Copy link
Contributor

@mattlavallee mattlavallee commented Sep 4, 2018

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:

@mattlavallee mattlavallee changed the title fix(#8740): clean up memory leak after loading async component completes fix #8740: clean up memory leak after loading async component completes Sep 4, 2018
@@ -457,7 +457,7 @@ if (!isIE9) {
setTimeout(() => {
resolve({ template: '<div><h1>component B</h1></div>' })
Vue.nextTick(next)
}, (duration + buffer) * 1.5)
}, (duration + buffer) * 1.7)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was noticing timing issues while running the vue tests on my laptop locally. This test would pass if part of the entire suite, but if it was marked as a focused test, it would fail due to timing issues. I encountered this with the code prior to my change as well

@mattlavallee mattlavallee changed the title fix #8740: clean up memory leak after loading async component completes fix(component): clean up memory leak after loading async component completes (fix #8740) Sep 4, 2018
}

const currContext = contexts[i]
const contextIdx = contexts.indexOf(currContext)
Copy link

Choose a reason for hiding this comment

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

isn't contextInx === i?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wa3l - good catch. The correct value of i is preserved when used in $nextTick if I just assign it to contextIdx.
I have updated the diff to include this refactor

@beyond8512
Copy link

是是是

@yyx990803 yyx990803 merged commit 2e472c5 into vuejs:dev Nov 30, 2018
f2009 pushed a commit to f2009/vue that referenced this pull request Jan 25, 2019
…mpletes (fix vuejs#8740) (vuejs#8755)

* fix(component): clean up memory leak after loading async component completes

* fix(async component): accounting for async components with loading property

* refactor(component): simplifying memory cleanup logic
aJean pushed a commit to aJean/vue that referenced this pull request Aug 19, 2020
…mpletes (fix vuejs#8740) (vuejs#8755)

* fix(component): clean up memory leak after loading async component completes

* fix(async component): accounting for async components with loading property

* refactor(component): simplifying memory cleanup logic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants