Skip to content

Conversation

MR-fisher
Copy link

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:
You can test in this case https://github.com/MR-fisher/vue-memory-leak-case.

vm._isBeingDestroyed = false
}

let releaseStack = new WeakMap()
Copy link
Member

Choose a reason for hiding this comment

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

we cannot use a weakmap as it would break support for IE

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your code review, I had coding WeakMap in another way to support lower version IE.

@posva
Copy link
Member

posva commented Jan 17, 2019

Hey, I appreciate that you created a custom Map to handle that but that would be too much. I didn't review the memory leak nor the PR (completely yet). I don't want you to waste more time on partial reviews so I'll do a full review when I can :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants