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 slot leak #677

Closed
wants to merge 33 commits into from
Closed

Fix slot leak #677

wants to merge 33 commits into from

Conversation

bigopon
Copy link
Member

@bigopon bigopon commented Jan 19, 2020

Fixes aurelia/templating-resources/issues/392

Thanks to @thomas-darling for this fix.


Currently, ShadowSlot keeps track of what Node has been projected through it, via an internal children array but it doesn't untrack(remove) those tracked Node from the array when the owning views of those Nodes get removed. Fix this by removing them in ShadowSlot.prototype.removeView and ShadowSlot.prototype.removeAll

Also revamp testing setup so that it can remove the usage of jspm for testing, while keeping the build process intact. Now we have:

npm run test
npm run test:watch
npm run test:debugger

with an option to go full integration tests to have more solid infra for tricky parth such as Shadow DOM emulation.

@EisenbergEffect @fkleuver @StrahilKazlachev

@bigopon bigopon changed the base branch from master to develop January 19, 2020 13:36
@claassistantio
Copy link

claassistantio commented Jan 19, 2020

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ baerrach
✅ bigopon
❌ AureliaEffect
You have signed the CLA already but the status is still pending? Let us recheck it.

@bigopon
Copy link
Member Author

bigopon commented Jan 19, 2020

@fkleuver @EisenbergEffect @StrahilKazlachev ready for review

@StrahilKazlachev
Copy link
Contributor

@bigopon I've not been active for some time, so are dist files added now with the PRs?

@bigopon
Copy link
Member Author

bigopon commented Jan 19, 2020

@StrahilKazlachev its because the target branch is develop and the pr is based on master. The commits that contain those dist should ideally ve in develop already. Cc @fkleuver

@bigopon
Copy link
Member Author

bigopon commented Jan 22, 2020

resolved by #678

@bigopon bigopon closed this Jan 22, 2020
@bigopon bigopon deleted the fix-slot-leak branch January 22, 2020 12:27
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.

None yet

5 participants