-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Do not reset global.document before CustomElement:disconnectedCallbac… #11871
Do not reset global.document before CustomElement:disconnectedCallbac… #11871
Conversation
Codecov Report
@@ Coverage Diff @@
## main #11871 +/- ##
=======================================
Coverage 68.93% 68.93%
=======================================
Files 312 312
Lines 16412 16412
Branches 4760 4760
=======================================
Hits 11314 11314
Misses 5071 5071
Partials 27 27
Continue to review full report at Codecov.
|
@dalvarezmartinez1 Thanks for sending a PR! Could you add a test that fails without this change and a changelog entry? 🙂 |
@dalvarezmartinez1 ping 🙂 |
It's on my list, I've been crazy busy lately, please don't close it, I will eventually do it |
8363f1e
to
84962fa
Compare
…k has finished running, document should be accessible in this callback function
84962fa
to
066e22b
Compare
@SimenB added test and changelog entry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
The order of the statements inside of the teardown function was changed.
This is due to document not being available in the disconnectedCallback function of CustomElements.
The stack trace looks something like this:
this.global.close(); -> window.js::close -> Element.js::innerHTML -> custom-elements.js::ceReactionsPostSteps -> custom-elements.js::invokeCEReactions -> Function.js::invokeTheCallbackFunction -> my-element::disconnectedCallback
Afaik, the document should be available inside of disconnectedCallback, if it's not this results in a:
"Error: Uncaught [TypeError: Cannot read property 'dispatchEvent' of null]"
Test plan
I ran yarn build and yarn test. Something weird: during the yarn test, it was written 1 test failed but at the end all succeeded.