clipboard-polyfill icon indicating copy to clipboard operation
clipboard-polyfill copied to clipboard

Upgrade TypeScript and drop DOM global types

Open aduth opened this issue 3 years ago • 5 comments

TypeScript now ships with type definitions for clipboard globals in the DOM lib as of TypeScript 4.4. This pull request attempts to fix errors which can occur when trying to use clipboard-polyfill in combination with TypeScript 4.4 and newer, where there are conflicts between the two sets of type definitions.

The proposed solution here is to remove the project's own type definitions, deferring instead to TypeScript built-in DOM types.

This could be considered a breaking change, as it would require the consuming project to use TypeScript 4.4 or newer and opt-in to DOM lib types.

Click to expand error
node_modules/clipboard-polyfill/dist/overwrite-globals/targets/overwrite-globals.d.ts:4:11 - error TS2451: Cannot redeclare block-scoped variable 'ClipboardItem'.

4 const ClipboardItem: ClipboardItemConstructor; ~~~~~~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:3495:11 3495 interface ClipboardItem { ~~~~~~~~~~~~~ 'ClipboardItem' was also declared here. node_modules/typescript/lib/lib.dom.d.ts:3500:13 3500 declare var ClipboardItem: { ~~~~~~~~~~~~~ and here.

node_modules/typescript/lib/lib.dom.d.ts:3473:11 - error TS2430: Interface 'Clipboard' incorrectly extends interface 'import("/project/node_modules/clipboard-polyfill/dist/overwrite-globals/ClipboardItem/spec").Clipboard'. The types returned by 'read()' are incompatible between these types. Type 'Promise<ClipboardItems>' is not assignable to type 'Promise<import("/project/node_modules/clipboard-polyfill/dist/overwrite-globals/ClipboardItem/spec").ClipboardItems>'. Type 'ClipboardItems' is not assignable to type 'import("/project/node_modules/clipboard-polyfill/dist/overwrite-globals/ClipboardItem/spec").ClipboardItems'. Type 'ClipboardItem' is not assignable to type 'ClipboardItemInterface'. Types of property 'types' are incompatible. The type 'readonly string[]' is 'readonly' and cannot be assigned to the mutable type 'string[]'.

3473 interface Clipboard extends EventTarget { ~~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:3495:11 - error TS2451: Cannot redeclare block-scoped variable 'ClipboardItem'.

3495 interface ClipboardItem { ~~~~~~~~~~~~~

node_modules/clipboard-polyfill/dist/overwrite-globals/targets/overwrite-globals.d.ts:4:11 4 const ClipboardItem: ClipboardItemConstructor; ~~~~~~~~~~~~~ 'ClipboardItem' was also declared here.

node_modules/typescript/lib/lib.dom.d.ts:3500:13 - error TS2451: Cannot redeclare block-scoped variable 'ClipboardItem'.

3500 declare var ClipboardItem: { ~~~~~~~~~~~~~

node_modules/clipboard-polyfill/dist/overwrite-globals/targets/overwrite-globals.d.ts:4:11 4 const ClipboardItem: ClipboardItemConstructor; ~~~~~~~~~~~~~ 'ClipboardItem' was also declared here.

aduth avatar Feb 07 '22 19:02 aduth

I guess this is what I sign up for with such a library. Thanks for making all these changes!

I can't promise to look at it super soon, but I think TypeScript 4.4 support is still valuable even through this library is just for legacy compat at this point. I'll probably give clipboard-polyfill a major version bump as a compat signal.

lgarron avatar Feb 07 '22 20:02 lgarron

Would you mind also sharing a repro where e.g. npx tsc runs into the given error?

lgarron avatar Feb 07 '22 21:02 lgarron

Would you mind also sharing a repro where e.g. npx tsc runs into the given error?

Sure, here's a sample repository:

https://github.com/aduth/tmp-repro-clipboard-typescript

It looks like the issue might be limited to the overwrite-globals entrypoint? At least, this is how we're using it, and I didn't see the same error when importing from the base clipboard-polyfill.

aduth avatar Feb 08 '22 02:02 aduth

Alright, I gave it a go on this branch: https://github.com/lgarron/clipboard-polyfill/tree/typescript-4.5.5

I want to avoid requiring TypeScript 4.4, since the whole point of this project is (now) legacy compat. I can't seem to get typesVersion to work and TypeScript seems to have a lot of arbitrary quirks around browser globals, so TypeScript < 4.4 may have to use window.ClipboardItem instead of ClipboardItem but that is a relatively mild drop-in change (the clipboard API is not available in workers).

Could you let me know if that branch works for you?

lgarron avatar Feb 08 '22 10:02 lgarron

Thanks for taking a look at this so quickly. I tried checking out the new branch, though unfortunately I wasn't able to generate the type definitions due to the following errors:

$ npx tsc
node_modules/typescript/lib/lib.dom.d.ts:3473:11 - error TS2430: Interface 'Clipboard' incorrectly extends interface 'import("/clipboard-polyfill/src/ClipboardItem/spec").Clipboard'.
  The types returned by 'read()' are incompatible between these types.
    Type 'Promise<ClipboardItems>' is not assignable to type 'Promise<import("/clipboard-polyfill/src/ClipboardItem/spec").ClipboardItems>'.
      Type 'ClipboardItems' is not assignable to type 'import("/clipboard-polyfill/src/ClipboardItem/spec").ClipboardItems'.
        Type 'ClipboardItem' is not assignable to type 'ClipboardItemInterface'.
          Types of property 'types' are incompatible.
            The type 'readonly string[]' is 'readonly' and cannot be assigned to the mutable type 'string[]'.

3473 interface Clipboard extends EventTarget {
               ~~~~~~~~~

node_modules/typescript/lib/lib.dom.d.ts:3495:11 - error TS2451: Cannot redeclare block-scoped variable 'ClipboardItem'.

3495 interface ClipboardItem {
               ~~~~~~~~~~~~~

  typesVersions/overwrite-globals-pre4.4.d.ts:38:9
    38   const ClipboardItem: ClipboardItemConstructor;
               ~~~~~~~~~~~~~
    'ClipboardItem' was also declared here.

node_modules/typescript/lib/lib.dom.d.ts:3500:13 - error TS2451: Cannot redeclare block-scoped variable 'ClipboardItem'.

3500 declare var ClipboardItem: {
                 ~~~~~~~~~~~~~

  typesVersions/overwrite-globals-pre4.4.d.ts:38:9
    38   const ClipboardItem: ClipboardItemConstructor;
               ~~~~~~~~~~~~~
    'ClipboardItem' was also declared here.


Found 3 errors.

aduth avatar Feb 08 '22 17:02 aduth

I've removed all global types in v4.0.0-rc3 because they cause more trouble than they're worth, and should no longer be needed for any reasonably modern project. (And if you're not in a reasonably modern project, there are always other builds available or you can define your own globals.)

Please let me know if v4.0.0-rc3 works for you!

lgarron avatar Jan 05 '23 09:01 lgarron

Hey @lgarron , thanks for picking this back up. I just tested and it appears that v4.0.0-rc3 is working as expected!

aduth avatar Jan 06 '23 13:01 aduth

@lgarron I may have spoken too soon, as I'm seeing some errors about "No "exports" main defined in clipboard-polyfill/package.json". I assume it's not related to the TypeScript types though. I'll dig a bit more and see what I can find.

Edit: I'm thinking this has to do with how the new version is ESM-only, and our project is still using CommonJS for our Mocha-based tests. If that's expected, maybe not an "issue" as much as me needing to upgrade our project.

aduth avatar Jan 06 '23 15:01 aduth

Hmm, I've removed the main field in favor of module, but maybe it'll work if I add it back. (That's a pretty weirdly worded error message, though.)

I don't plan to support non-ESM builds, though, so if that's the issue you'll have to adapt your stack.

lgarron avatar Jan 06 '23 22:01 lgarron

Alright, please see if v4.0.0-rc6 works for you!

lgarron avatar Jan 06 '23 22:01 lgarron

@lgarron Thanks for that revision. I think it could make sense to include the main, though it doesn't help for our case, which does appear to be more to do with the fact that we're still relying on CommonJS (the error message is probably referring to the fact that none of the "exports" entrypoints are resolvable for CommonJS). That's a bigger effort than I'll likely be able to take on for the short-term, though I don't think that ought to have any impact on your project (aside perhaps a compatibility note in the release notes).

I may try to see if I can get it working with dynamic import() as a workaround.

aduth avatar Jan 09 '23 14:01 aduth

Yeah, unfortunately we no longer provide CommonJS builds. I have some advice at https://github.com/lgarron/clipboard-polyfill#bundling--tree-shaking--minification--commonjs if you want to get a CommonJS build, although it might be easier for you to pin to v3.0.3.

lgarron avatar Jan 09 '23 19:01 lgarron