-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
🚀♻️ Custom Elements V1 Polyfill #17205
Changes from 23 commits
8b525a7
94d836a
f09eebf
c6f0c7e
ad2c0eb
68cf92f
01d7530
70a70ff
dc51611
f821d00
35ebc6d
b5072fc
16ac093
6b7a687
ba171aa
eaf7422
873f0b0
81da198
e3eeafe
72ab68f
50660bb
6a7477b
ecab91f
f46cee0
036cf0d
4d47bff
005637d
6653195
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,20 +14,25 @@ | |
* limitations under the License. | ||
*/ | ||
|
||
import {getMode} from './mode'; | ||
import {install as installArrayIncludes} from './polyfills/array-includes'; | ||
import {install as installCustomElements} from './polyfills/custom-elements'; | ||
import { | ||
install as installDOMTokenListToggle, | ||
} from './polyfills/domtokenlist-toggle'; | ||
import {install as installDocContains} from './polyfills/document-contains'; | ||
import {install as installMathSign} from './polyfills/math-sign'; | ||
import {install as installObjectAssign} from './polyfills/object-assign'; | ||
import {install as installPromise} from './polyfills/promise'; | ||
// Importing the document-register-element module has the side effect | ||
// of installing the custom elements polyfill if necessary. | ||
import {installCustomElements} from | ||
import {isExperimentOn} from './experiments'; | ||
import {installCustomElements as registerElement} from | ||
'document-register-element/build/document-register-element.patched'; | ||
|
||
installCustomElements(self, 'auto'); | ||
if (isExperimentOn(self, 'custom-elements-v1') || getMode().test) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You want at least one test that ensures the old still works. That is unless you are super optimistic about moving over. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm more worried about bugs with using native custom elements than I am with this polyfill not matching the current polyfill. Worst case we move to this one, and disable the native CE/ |
||
installCustomElements(self, class {}); | ||
} else { | ||
registerElement(self, 'auto'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Confusing since this looks like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
} | ||
installDOMTokenListToggle(self); | ||
installMathSign(self); | ||
installObjectAssign(self); | ||
|
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.
Did the maxSize increase because we are shipping both polyfills in this PR?
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.
Yup.