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

Popper: Fix memory leak on route change (#7715) #9757

Merged
merged 1 commit into from
Feb 11, 2018
Merged

Popper: Fix memory leak on route change (#7715) #9757

merged 1 commit into from
Feb 11, 2018

Conversation

a-kriya
Copy link
Contributor

@a-kriya a-kriya commented Feb 9, 2018

Fixes #7715 by addressing the two following issues:

  • When the route changed on click, the doDestroy method of vue-popper was returning right away without actually destroying the popper due to showPopper being true. forceDestroy parameter is added to handle this case. An alternative fix could be to set showPopper to false in beforeDestroy before calling doDestroy, but the parameter approach was chosen as it can be reused.
  • After the above fix, the destroy method of popper.js was being successfully called, which in turn called _removeEventListeners. The target that this method was removing the scroll event listener from was not the same element to which the listener was added (this is due to getScrollParent not being able to find the correct element as some of the top level nodes had already been destroyed). This caused the event listener (bound to this) to leak, and consequently the whole associated DOM and VDOM tree. The fix is to replace the logic to find the target in _removeEventListeners and use the stored target to remove the listener from.
  • Make sure you follow Element's contributing guide (中文 | English | Español).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer relative issues for you PR.

@a-kriya a-kriya changed the title Popper: Fix memory leak on route change Popper: Fix memory leak on route change (#7715) Feb 9, 2018
@a-kriya
Copy link
Contributor Author

a-kriya commented Feb 9, 2018

Second issue:

popper_out

With the fix:

fix

@Leopoldthecoder Leopoldthecoder merged commit 1b3832f into ElemeFE:dev Feb 11, 2018
@a-kriya a-kriya deleted the 7715 branch February 11, 2018 04:31
bluejfox added a commit to setariajs/setaria-ui that referenced this pull request Apr 16, 2018
Merge commit 'ec30f9c67d30522d3d054a8433abf5934df90fe7' into current

* commit 'ec30f9c67d30522d3d054a8433abf5934df90fe7': (44 commits)
  Chore: update package.json to lock lint config and fix console lint errors (ElemeFE#9773)
  Chore: update contribution guide (ElemeFE#9803)
  Doc: update changelogs (ElemeFE#9802)
  Menu: update es doc (ElemeFE#9801)
  Docs: update es docs for 2.2.0 (ElemeFE#9796)
  [release] 2.2.0
  [build] 2.2.0
  Changelog: update for 2.2.0 (ElemeFE#9793)
  Select: add popper-append-to-body (ElemeFE#9782)
  ColorPicker: declare box-sizing for panel (ElemeFE#9780)
  Chore: replace var with let and const (ElemeFE#9774)
  Menu: improve vertical collapse transition (ElemeFE#9777)
  support menu disabled (ElemeFE#9771)
  Popper: fix memory leak on route change (ElemeFE#9757)
  i18n: add Kurdish translation (ElemeFE#9582)
  CheckBox: hide the native input using z-index (ElemeFE#9746)
  Switch: fix click event triggering twice (ElemeFE#9760)
  Transfer: add clearQuery (ElemeFE#9753)
  Chore: update docs for Tree and Menu (ElemeFE#9751)
  Menu: support multi level submenu in horizontal mode (ElemeFE#9741)
  ...

# Conflicts:
#	CHANGELOG.zh-CN.md
#	README.md
#	build/bin/version.js
#	build/deploy-ci.sh
#	examples/versions.json
#	package.json
#	packages/tree/src/tree-node.vue
#	src/index.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug Report] Memory leak on route change with a visible tooltip
4 participants