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

[TypeScript] Possible breakage of LegacyElementMixin type definition with the release of 3.4.0 (No problem on 3.3.1) #5651

Closed
petersaints opened this issue Apr 28, 2020 · 4 comments

Comments

@petersaints
Copy link

Steps to Reproduce

  1. Import a Polymer component in TypeScript with an interface that extends both LegacyElementMixin and HTMLElement (e.g., https://www.webcomponents.org/element/@polymer/paper-dialog).
    import { PaperDialogElement } from '@polymer/paper-dialog/paper-dialog.js';
  2. Try to compile your project.
#### Expected Results
No error during compilation.

#### Actual Results
The following errors occur:
node_modules/@polymer/paper-dialog/paper-dialog.d.ts:78:11 - error TS2320: Interface 'PaperDialogElement' cannot simultaneously extend types 'LegacyElementMixin' and 'HTMLElement'.
  Named property 'removeAttribute' of types 'LegacyElementMixin' and 'HTMLElement' are not identical.

78 interface PaperDialogElement extends PaperDialogBehavior, NeonAnimationRunnerBehavior, LegacyElementMixin, HTMLElement {
             ~~~~~~~~~~~~~~~~~~

node_modules/@polymer/paper-dialog/paper-dialog.d.ts:78:11 - error TS2320: Interface 'PaperDialogElement' cannot simultaneously extend types 'LegacyElementMixin' and 'HTMLElement'.
  Named property 'setAttribute' of types 'LegacyElementMixin' and 'HTMLElement' are not identical.

78 interface PaperDialogElement extends PaperDialogBehavior, NeonAnimationRunnerBehavior, LegacyElementMixin, HTMLElement {
             ~~~~~~~~~~~~~~~~~~


Found 2 errors.

I looked a bit into it and it seems that on Polymer v3.3.1 there were no removeAttribute and setAttribute methods on the LegacyElementMixin interface, whereas they are present in v3.4.0.

That wouldn't be a problem, but the method signature doesn't match the one found on HTMLElement so when an interface tries to extend both of those at the same they will clash.

On HTMLElement you have the following signatures:

removeAttribute(qualifiedName: string): void;
setAttribute(qualifiedName: string, value: string): void;
On version 3.4.0's LegacyElementMixin you have these signatures:
setAttribute(name: any, value: any): void;
removeAttribute(name: any): void;

As you can see, they are not pefect matches. string should be used instead of any.

I don't know exactly how do you generate your type definitions, since the library is not written in TypeScript. But you are probably doing it by extracting type information from JSDoc.

So could you please fix this small issue and make a quick v3.4.1 release?

Thanks in advance.

Browsers Affected

  • Not applicable, this is a build time issue.

Versions

  • @polymer/lit-element: v2.3.1
  • @polymer/paper-dialog: v3.0.1
  • @polymer/polymer: v3.4.0
@robrez
Copy link

robrez commented Apr 28, 2020

Came to report the same thing, one of our CI pipelines broke today. Confirmed that pinning polymer ~3.3.0 fixes the issues

../../node_modules/@polymer/paper-icon-button/paper-icon-button.d.ts(75,11): error TS2320: Interface 'PaperIconButtonElement' cannot simultaneously extend types 'LegacyElementMixin' and 'HTMLElement'.
  Named property 'removeAttribute' of types 'LegacyElementMixin' and 'HTMLElement' are not identical.
../../node_modules/@polymer/paper-ripple/paper-ripple.d.ts(86,11): error TS2320: Interface 'PaperRippleElement' cannot simultaneously extend types 'LegacyElementMixin' and 'HTMLElement'.
  Named property 'setAttribute' of types 'LegacyElementMixin' and 'HTMLElement' are not identical.
../../node_modules/@polymer/paper-spinner/paper-spinner.d.ts(52,11): error TS2320: Interface 'PaperSpinnerElement' cannot simultaneously extend types 'LegacyElementMixin' and 'HTMLElement'.
  Named property 'removeAttribute' of types 'LegacyElementMixin' and 'HTMLElement' are not identical.

@robrez
Copy link

robrez commented Apr 28, 2020

Looks like this PR maybe the intended fix: #5650

It came not long after v3.4.0 was released

@bicknellr bicknellr self-assigned this Apr 28, 2020
@bicknellr
Copy link
Member

bicknellr commented Apr 28, 2020

Thanks for reporting! Yes, #5650 should fix this but I'm currently working some issues importing it internally since it goes through a different type checker there. Once that's fixed, I'll make a v3.4.1 release.

@bicknellr
Copy link
Member

Sorry about the wait, turnaround time for those tests is kind of long and we had to do a few cycles. v3.4.1 is now released and should resolve this issue; let me know here if you're still experiencing this issue after upgrading. Again, thanks for reporting!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants