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

Add toggleAttribute to excluded identifiers #5371

Closed
wants to merge 1 commit into from

Conversation

keanulee
Copy link
Contributor

@keanulee keanulee commented Oct 8, 2018

TypeScript 3.1 added Element.toggleAttribute(name, force?) to its DOM typings which is compatible but not identical to LegacyElementMixin.toggleAttribute(name, force?, element?)

#5370 fixes the compatibility issue, but tsc still errors:

node_modules/@polymer/app-layout/app-drawer/app-drawer.d.ts:67:11 - error TS2320: Interface 'AppDrawerElement' cannot simultaneously extend types 'LegacyElementMixin' and 'HTMLElement'.
  Named property 'toggleAttribute' of types 'LegacyElementMixin' and 'HTMLElement' are not identical.

67 interface AppDrawerElement extends LegacyElementMixin, HTMLElement {
             ~~~~~~~~~~~~~~~~

This patch removes the custom typing of toggleAttribute(). We did this before with scroll() in iron-scroll-target-behavior (PolymerElements/iron-scroll-target-behavior#48).

TypeScript 3.1 added `Element.toggleAttribute(name, force?)` to its DOM typings which is compatible but not identical to `LegacyElementMixin.toggleAttribute(name, force?, element?)`
@TimvdLippe
Copy link
Contributor

This is a breaking change for users on TypeScript 2. I am confused though, as @43081j indicated he saw the issue was resolved. @43081j do you mind elaborating upon this PR?

@43081j
Copy link
Contributor

43081j commented Oct 9, 2018

Interesting, in the project i tried it on, I was using a few paper-* elements and they seemed to compile fine.

I'll take a look and get back to it

@keanulee
Copy link
Contributor Author

keanulee commented Oct 9, 2018

@43081j Specifically, I had issues with npm run generate-types on app-layout after bumping to TS 3.1. First error:

Compilation failed
app-box/app-box.d.ts:93:11 - error TS2320: Interface 'AppBoxElement' cannot simultaneously extend types 'LegacyElementMixin' and 'HTMLElement'.
  Named property 'toggleAttribute' of types 'LegacyElementMixin' and 'HTMLElement' are not identical.

93 interface AppBoxElement extends AppScrollEffectsBehavior, IronResizableBehavior, LegacyElementMixin, HTMLElement {
             ~~~~~~~~~~~~~

@43081j
Copy link
Contributor

43081j commented Oct 9, 2018

yup this is reproducible here.

strangely my local project which uses paper-dialog builds fine with the type change (from void to bool).

still investigating why mine builds and the TS repro doesn't.

@43081j
Copy link
Contributor

43081j commented Oct 11, 2018

🙈

@TimvdLippe it turns out skipLibCheck had slipped into my tsconfig. I can now reproduce this locally too.

Is it possible we could output the built in signature but still call it in JS with the extra parameter? It sounds hacky... but can't think of a better solution

@TimvdLippe
Copy link
Contributor

TimvdLippe commented Oct 11, 2018 via email

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.

4 participants