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

Upgrade TypeScript and drop DOM global types #148

Closed
wants to merge 1 commit into from

Conversation

aduth
Copy link

@aduth aduth commented Feb 7, 2022

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' 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.

TypeScript now ships with type definitions for clipboard globals in the DOM lib as of TypeScript 4.4. This fixes 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 type definitions.
@lgarron
Copy link
Owner

lgarron commented Feb 7, 2022

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
Copy link
Owner

lgarron commented Feb 7, 2022

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

@aduth
Copy link
Author

aduth commented Feb 8, 2022

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.

@lgarron
Copy link
Owner

lgarron commented Feb 8, 2022

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?

@aduth
Copy link
Author

aduth commented Feb 8, 2022

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.

@lgarron lgarron closed this Apr 11, 2022
@lgarron lgarron reopened this Apr 11, 2022
@lgarron lgarron force-pushed the main branch 3 times, most recently from ba6ef45 to 49dd676 Compare January 5, 2023 09:08
lgarron added a commit that referenced this pull request Jan 5, 2023
Global augmentation in TypeScript is still troublesome. In particular, it's difficult to exclude (and therefore difficult to avoid compile errors) if it's available in a project or `node_modules`, even if not imported directly:

- microsoft/TypeScript#37053
- #148
@lgarron
Copy link
Owner

lgarron commented Jan 5, 2023

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 lgarron closed this Jan 5, 2023
@aduth aduth deleted the aduth-typescript-4-4-compat branch January 6, 2023 13:25
@aduth
Copy link
Author

aduth commented Jan 6, 2023

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

@aduth
Copy link
Author

aduth commented Jan 6, 2023

@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.

@lgarron
Copy link
Owner

lgarron commented Jan 6, 2023

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 added a commit that referenced this pull request Jan 6, 2023
@lgarron
Copy link
Owner

lgarron commented Jan 6, 2023

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

@aduth
Copy link
Author

aduth commented Jan 9, 2023

@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.

@lgarron
Copy link
Owner

lgarron commented Jan 9, 2023

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.

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

Successfully merging this pull request may close these issues.

2 participants