-
Notifications
You must be signed in to change notification settings - Fork 165
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
Convert the Custom Elements polyfill to TypeScript #246
Conversation
#247 should fix the build error. |
2c3179b
to
d9a57f8
Compare
Cool, tests are passing now, PTAL |
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.
WDYT about using Array<T>
throughout instead of T[]
? AFAICT, Array<X|Y>
seems to be preferred over (X|Y)[]
, but then T[]
is used instead of Array<T>
when T
is a single identifier - is there a reason for mixing them?
Found a few places that are now ??
/?.
-able while reading through this again but I think it's not worth worrying about for this PR.
TypeScript doesn't care, but it does make it invalid JSON.
92e8ca4
to
9ba0df2
Compare
Err, LGTM once we get the tests working, but it looks like you just need to be added as admin or collaborator on this repo? |
Sauce tests passing in #268 but the discussion is here, so I'll merge this change and close that one |
Potentially breaking change: the individual source files (including native-shim.js) are now in lib instead of src. If that would break users I can refactor to move them back to
src/
Also, for sanity, I enabled automatic formatting. As a result, the changes are probably best reviewed one commit at a time.