maxisam / ngx-clipboard

A pure angular clipboard directive
http://maxisam.github.io/ngx-clipboard/
MIT License
505 stars 93 forks source link

strictTemplates warns about ngxClipboard #239

Closed mircowidmer closed 4 years ago

mircowidmer commented 4 years ago

When I enable "strictTemplates": true in my angular compiler options, it warns me about the second line:

    <button ngxClipboard
            [cbContent]="'any content'"></button>

error TS2322: Type 'undefined' is not assignable to type 'HTMLInputElement | HTMLTextAreaElement'.

I'm using Angular 9 and ngxClipboard 13.

maxisam commented 4 years ago

have you tried skipLibCheck = true?

maxisam commented 4 years ago

https://github.com/maxisam/ngx-clipboard/blob/3b1cb50a8ae6108bfa94e16174679a5279926825/projects/ngx-clipboard/src/lib/ngx-clipboard.directive.ts#L12

I think it is caused by this line.

probably should be public targetElm: HTMLInputElement | HTMLTextAreaElement | undefined

maxisam commented 4 years ago

could you test 13.0.1-beta01?

mircowidmer commented 4 years ago

Unfortunately I can't reproduce the issue either with 13.0.0 or 13.0.1-beta01 in a vanilla angular application project.

I'm using

"angularCompilerOptions": {
    "fullTemplateTypeCheck": true,
    "strictTemplates": true,
    "strictInjectionParameters": true
  },

and

    "strict": true,
    "skipLibCheck": true

and can't reproduce the problem with either 13.0.0 or 13.01.-beta01. Somehow the two projects differ in another area as well :/

maxisam commented 4 years ago

ah... that is odd. Well, did you test 13.0.1-beta01 in your original proj

mircowidmer commented 4 years ago

Yes.

maxisam commented 4 years ago

And does it work?

mircowidmer commented 4 years ago

In my original project, the error TS2322 still occurs. I ended up using a simple custom-made directive for now. Thanks for the support!

maxisam commented 4 years ago

That's odd. I don't have better idea now.

mircowidmer commented 4 years ago

That's perfectly ok. I hope your fix still benefits the project. You can close the issue if you'd like. Maybe I'll get around to check it out more detailed in the future.

maxisam commented 4 years ago

I will leave it open for a while to see if anyone experience the same thing.

If you somehow can reproduce in a simple testing project. Maybe I can fix it.

maxisam commented 4 years ago

Also, make sure your typescript version is the latest support version. I think this issue might be related to typescript

mircowidmer commented 4 years ago

Ok, I was able to reproduce the problem with a very minimal example project, see https://github.com/mircowidmer/ngx-clipboard

maxisam commented 4 years ago

ok I think the problem is strictTemplates mode. This is only for Ivy but the library is not built in Ivy for now according to Angular team's suggestion. I will continue to monitor this issue. But currently I don't know how to fix it.

kmaraz commented 4 years ago

Hi, it happened to me also.

This is the UGLY HACK I have used:

[ngxClipboard]="$any('')"

It works, but it is very ugly.

DmitryEfimenko commented 4 years ago

+1 to this. The issue indeed has to do with the combination of enabling strict template checks and the type of the element the directive ngxClipboard is used on. Currently it's only allowing to be used on HTMLInputElement | HTMLTextAreaElement:

public targetElm: HTMLInputElement | HTMLTextAreaElement;

This will fail when using this directive on any other elements (the version 13.0.1-beta01 fails too). For example, I have a use-case:

<div ngxClipboard cbContent="something to copy"></div>

So clicking on a div should copy the provided content. However, this won't compile. To fix this, the targetElm should simply allow any HTMLElement

public targetElm: HTMLElement;

Although I'm wondering if this is still too restrictive. What if somebody wants to use this directive on an Angular component? As far as I know these are not of type HTMLElement. Also not sure about the use-case of using the directive on a web component (custom element).

Perhaps giving it type any is a safest way to go:

public targetElm: any;
Maximaximum commented 4 years ago

I'm also experiencing the same issue.

However, strictly speaking, it has nothing to do with the fullTemplateTypeCheck settings. It's just that having fullTemplateTypeCheck enabled brings this issue to light and lets compiler emit an error, whereas without fullTemplateTypeCheck the issue simply gets ignored by the Angular compiler.

Anyway, the real underlying issue is that ngx-clipboard is compiled without using the typescript's strictNullChecks option. Using this option is often considered to be a best practice for creating Angular libraries. IMHO, however, it's an absolute MUST for ANY public Angular library to use it. Unfortunately, having strictNullChecks disabled in the library project produces *.d.ts typing files which are completely unusable by applications that DO have strictNullChecks enabled.

So, the easiest way to fix this issue would be to turn the strictNullChecks option on in the tsconfig.json file of the ngx-clipboard project, and then update all the type annotations within ngx-clipboard to properly deal with undefined and null types.

Please let me know if any additional explanation/help needed, @maxisam. I really hope that this issue will get fixed soon.

maxisam commented 4 years ago

@Maximaximum Thanks! Please test v13.0.1-beta02.

Maximaximum commented 4 years ago

Hi @maxisam!

I've tried the beta, and I have to admit that, besides strictNullChecks there's actually an additional issue with strictTemplates. Seems like now your code is working fine with the undefined values, but it actually needs to support the '' value, because that's what <div ngxClipboard></div> actually resolves to.

So, now there are 2 options:

Personally, I'd rather go with the 2nd option, since IMHO it's a bit more clear and readable.

Maximaximum commented 4 years ago

Please also note that in order to test this you can simply create a new Angular 9 project and set these options in tsconfig.json:

  "angularCompilerOptions": {
    "fullTemplateTypeCheck": true,
    "strictTemplates": true
  }
"compilerOptions": {
    // other options
    ...
    "strictNullChecks": true
}
maxisam commented 4 years ago

@Maximaximum please try v13.0.1-beta03. It works on mine.

I ended up go with the option1 because I have never seen the usage of option2. Mind if you can share the reference?

Maximaximum commented 4 years ago

Ah yes, sorry, I forgot to add the link to the docs: https://angular.io/guide/template-typecheck#input-setter-coercion

This is a new feature introduced in Angular 9, along with the strict template type checking

Maximaximum commented 4 years ago

I've tested v13.0.1-beta03. The ngxClipboard (targetElm) works fine.

However, the similar technique should be applied to the whole project.

In particular, I'm getting an error while trying to bind [cbContent] to a value of type string | undefined. I guess that people might experience the same issue with other inputs as well.

I'd suggest you to read about strictNullChecks and update the project's codebase in conformance to this concept: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0.html#--strictnullchecks

maxisam commented 4 years ago

Please try v13.0.1-beta04

I forgot cbContent could be undefined.

Maximaximum commented 4 years ago

It seems to work just fine now, thank you.

Waiting for the official release!

maxisam commented 4 years ago

Thank you for your help! v13.0.1 is out. https://github.com/maxisam/ngx-clipboard/releases/tag/v13.0.1

Maximaximum commented 4 years ago

Thanks, @maxisam!