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

Convert the Custom Elements polyfill to TypeScript #246

Merged
merged 17 commits into from
Feb 11, 2020

Conversation

rictic
Copy link
Contributor

@rictic rictic commented Jan 17, 2020

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.

@bicknellr
Copy link
Collaborator

#247 should fix the build error.

@rictic
Copy link
Contributor Author

rictic commented Jan 22, 2020

Cool, tests are passing now, PTAL

packages/custom-elements/src/CustomElementRegistry.ts Outdated Show resolved Hide resolved
packages/custom-elements/src/custom-elements.ts Outdated Show resolved Hide resolved
packages/custom-elements/tsconfig.json Outdated Show resolved Hide resolved
packages/custom-elements/src/CustomElementInternals.ts Outdated Show resolved Hide resolved
packages/custom-elements/.clang-format Show resolved Hide resolved
packages/custom-elements/src/CustomElementInternals.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@bicknellr bicknellr left a 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.

packages/custom-elements/gulpfile.js Outdated Show resolved Hide resolved
packages/custom-elements/src/AlreadyConstructedMarker.ts Outdated Show resolved Hide resolved
packages/custom-elements/src/CustomElementInternals.ts Outdated Show resolved Hide resolved
packages/custom-elements/src/CustomElementInternals.ts Outdated Show resolved Hide resolved
packages/custom-elements/src/CustomElementRegistry.ts Outdated Show resolved Hide resolved
packages/custom-elements/src/Patch/Node.ts Outdated Show resolved Hide resolved
packages/custom-elements/src/native-shim.ts Outdated Show resolved Hide resolved
packages/custom-elements/src/native-shim.ts Outdated Show resolved Hide resolved
packages/custom-elements/src/native-shim.ts Outdated Show resolved Hide resolved
packages/custom-elements/src/native-shim.ts Outdated Show resolved Hide resolved
packages/custom-elements/tsconfig.json Outdated Show resolved Hide resolved
packages/custom-elements/ts_src/Externs.ts Outdated Show resolved Hide resolved
@bicknellr
Copy link
Collaborator

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?

@rictic
Copy link
Contributor Author

rictic commented Feb 11, 2020

Sauce tests passing in #268 but the discussion is here, so I'll merge this change and close that one

@bicknellr bicknellr merged commit 73f52e0 into webcomponents:master Feb 11, 2020
@rictic rictic deleted the ts-custom-elements branch February 11, 2020 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants