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

meta: upgrade to TypeScript 4.8 #4048

Merged
merged 5 commits into from
Aug 30, 2022
Merged

meta: upgrade to TypeScript 4.8 #4048

merged 5 commits into from
Aug 30, 2022

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Aug 27, 2022

TS team released a not-so-big update that flagged some inconsistency in some of our .d.ts files.

Fixes: #4053

@Murderlon
Copy link
Member

This seem to fix #4053, is that correct?

@@ -21,7 +21,7 @@ export type FileProgress = UppyUtils.FileProgress;
export type FileRemoveReason = 'removed-by-user' | 'cancel-all';

// Replace the `meta` property type with one that allows omitting internal metadata addFile() will add that
type UppyFileWithoutMeta<TMeta, TBody> = OmitKey<
type UppyFileWithoutMeta<TMeta extends IndexedObject<any>, TBody extends IndexedObject<any>> = OmitKey<
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid a lot of duplication here by creating a new type that already extends it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC here we are defining the type TMeta, so I don't think that's an option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If TMeta is always with extends IndexedObject<any>, it can become the new type. If there are still exceptions, we can create a new type with combines the two and use those where needed in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm defining TMeta here, I don't think I can "make it a new type", or I don't understand what you mean.

@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label Aug 29, 2022
@github-actions github-actions bot removed pending end-to-end tests safe to test Add this label on trustworthy PRs to spawn the e2e test suite labels Aug 29, 2022
@aduh95 aduh95 marked this pull request as draft August 29, 2022 16:17
@aduh95
Copy link
Contributor Author

aduh95 commented Aug 29, 2022

Let's wait for @typescript-eslint/eslint-plugin to update to avoid the linter hacks and the annoying CLI warning. EDIT: they've just released a new version.

@aduh95 aduh95 marked this pull request as ready for review August 30, 2022 08:22
@aduh95 aduh95 added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label Aug 30, 2022
@github-actions github-actions bot removed pending end-to-end tests safe to test Add this label on trustworthy PRs to spawn the e2e test suite labels Aug 30, 2022
@aduh95 aduh95 merged commit f7d0d5b into transloadit:main Aug 30, 2022
@aduh95 aduh95 deleted the upgrade-ts branch August 30, 2022 09:46
@github-actions github-actions bot mentioned this pull request Aug 30, 2022
github-actions bot added a commit that referenced this pull request Aug 30, 2022
| Package                   | Version | Package                   | Version |
| ------------------------- | ------- | ------------------------- | ------- |
| @uppy/angular             |   0.4.1 | @uppy/store-default       |   3.0.1 |
| @uppy/audio               |   1.0.1 | @uppy/store-redux         |   3.0.1 |
| @uppy/aws-s3              |   3.0.1 | @uppy/svelte              |   3.0.0 |
| @uppy/aws-s3-multipart    |   3.0.1 | @uppy/thumbnail-generator |   3.0.1 |
| @uppy/companion           |   4.0.1 | @uppy/transloadit         |   3.0.1 |
| @uppy/companion-client    |   3.0.1 | @uppy/tus                 |   3.0.1 |
| @uppy/core                |   3.0.1 | @uppy/utils               |   5.0.1 |
| @uppy/dashboard           |   3.0.1 | @uppy/webcam              |   3.1.0 |
| @uppy/react               |   3.0.1 | @uppy/xhr-upload          |   3.0.1 |
| @uppy/remote-sources      |   1.0.1 | uppy                      |   3.0.1 |

- @uppy/dashboard,@uppy/webcam: add nativeCameraFacingMode to Webcam and Dashboard (Artur Paikin / #4047)
- meta: upgrade to Jest 29 (Antoine du Hamel / #4049)
- @uppy/svelte: update peer dependencies (Antoine du Hamel / #4065)
- @uppy/react: useUppy: fix unmount on NextJS dev mode (Matt Jesuele / #4062)
- @uppy/vue: fix missing component in docs (Antoine du Hamel / #4063)
- @uppy/angular: fix compiler warning (Antoine du Hamel / #4064)
- meta: improve CI npm install time (Antoine du Hamel / #4058)
- meta: example: fix Angular example package name (Antoine du Hamel / #4060)
- meta: upgrade to TypeScript 4.8 (Antoine du Hamel / #4048)
- @uppy/core,@uppy/dashboard,@uppy/thumbnail-generator: update definition type files for TS 4.8 compatibility (Antoine du Hamel / #4055)
- @uppy/transloadit: improve deprecation notice (Antoine du Hamel / #4056)
- @uppy/thumbnail-generator: fix `exifr` import (Antoine du Hamel / #4054)
- @uppy/utils: fix `relativePath` when drag&dropping a folder (Antoine du Hamel / #4043)
- @uppy/companion: Fix Companion license (Merlijn Vos / #4044)
- e2e: add tests for AWS (Antoine du Hamel / #3665)
- meta: Only publish Companion to Dockerhub on release (Merlijn Vos / #4037)
- meta: fix linter warnings (Antoine du Hamel / #4039)
- @uppy/utils: Post-release website fixes (Merlijn Vos / #4038)
- @uppy/angular: fix peer dependencies (Antoine du Hamel / #4035)
- meta: uppy.io homepage: Add Tus (Artur Paikin)
- meta: Fix uppy.io homepage example (Artur Paikin)
@jlowcs jlowcs mentioned this pull request Aug 30, 2022
2 tasks
@jlowcs
Copy link

jlowcs commented Aug 30, 2022

Could this fix possibly be applied to uppy 2.x for those of us who can move to ESM quite yet? 🙏

@aduh95
Copy link
Contributor Author

aduh95 commented Aug 30, 2022

Could this fix possibly be applied to uppy 2.x for those of us who can move to ESM quite yet? 🙏

I've cherry-picked it onto the 2.x branch (4a74113), I don't know when it will be released as we just did a maintenance release for 2.x.

@jlowcs
Copy link

jlowcs commented Sep 27, 2022

Looks like there was a maintenance release a couple days ago but it didn't include this change?

@Murderlon
Copy link
Member

It looks like #4072 was cherry-picked, which was a fix for #4055, which were the changes required after this upgrade PR. I'll try to cherry-pick the other two too.

@Murderlon
Copy link
Member

Looking closer, it should be in there. Have you tested this?

@jlowcs
Copy link

jlowcs commented Sep 27, 2022

My dependabot PR for TS 4.8.3 is up to date and reports the following:

node_modules/@uppy/dashboard/types/index.d.ts:87:69 - error TS2344: Type 'TMeta' does not satisfy the constraint 'IndexedObject<any>'.

87 export type DashboardFileEditStartCallback<TMeta> = (file: UppyFile<TMeta>) => void;
                                                                       ~~~~~

  node_modules/@uppy/dashboard/types/index.d.ts:87:44
    87 export type DashboardFileEditStartCallback<TMeta> = (file: UppyFile<TMeta>) => void;
                                                  ~~~~~
    This type parameter might need an `extends IndexedObject<any>` constraint.

node_modules/@uppy/dashboard/types/index.d.ts:88:72 - error TS2344: Type 'TMeta' does not satisfy the constraint 'IndexedObject<any>'.

88 export type DashboardFileEditCompleteCallback<TMeta> = (file: UppyFile<TMeta>) => void;
                                                                          ~~~~~

  node_modules/@uppy/dashboard/types/index.d.ts:88:47
    88 export type DashboardFileEditCompleteCallback<TMeta> = (file: UppyFile<TMeta>) => void;
                                                     ~~~~~
    This type parameter might need an `extends IndexedObject<any>` constraint.

node_modules/@uppy/thumbnail-generator/types/index.d.ts:24:18 - error TS2344: Type 'TMeta' does not satisfy the constraint 'IndexedObject<any>'.

24   file: UppyFile<TMeta>,
                    ~~~~~

  node_modules/@uppy/thumbnail-generator/types/index.d.ts:23:40
    23 export type ThumbnailGeneratedCallback<TMeta> = (
                                              ~~~~~
    This type parameter might need an `extends IndexedObject<any>` constraint.

@Murderlon
Copy link
Member

That's very odd, these changes are on the 2.x branch:

export type DashboardFileEditStartCallback<TMeta extends IndexedObject<any>> = (file: UppyFile<TMeta>) => void;
export type DashboardFileEditCompleteCallback<TMeta extends IndexedObject<any>> = (file: UppyFile<TMeta>) => void;

We can also see these changes are on the latest release:

https://esm.sh/@uppy/dashboard@2.4.3/types/index.d.ts

Are you really sure you are on the latest version?

@jlowcs
Copy link

jlowcs commented Sep 28, 2022

Ah, it's in @uppy/dashboard. It's only an indirect dependency for me.

> npm list @uppy/dashboard
[...]
└─┬ @uppy/react@2.2.3
  └── @uppy/dashboard@2.4.2

No new version of @uppy/react was apparently released, which would explain why it wasn't bumped by dependabot.

I managed to bump the indirect dependency.

@Murderlon
Copy link
Member

@uppy/react doesn't need a new version in order to get the latest @uppy/dashboard because of the semver range we use, only a reinstall is required :)

@jlowcs
Copy link

jlowcs commented Sep 28, 2022

confirming that it fixed the issue

HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
| Package                   | Version | Package                   | Version |
| ------------------------- | ------- | ------------------------- | ------- |
| @uppy/angular             |   0.4.1 | @uppy/store-default       |   3.0.1 |
| @uppy/audio               |   1.0.1 | @uppy/store-redux         |   3.0.1 |
| @uppy/aws-s3              |   3.0.1 | @uppy/svelte              |   3.0.0 |
| @uppy/aws-s3-multipart    |   3.0.1 | @uppy/thumbnail-generator |   3.0.1 |
| @uppy/companion           |   4.0.1 | @uppy/transloadit         |   3.0.1 |
| @uppy/companion-client    |   3.0.1 | @uppy/tus                 |   3.0.1 |
| @uppy/core                |   3.0.1 | @uppy/utils               |   5.0.1 |
| @uppy/dashboard           |   3.0.1 | @uppy/webcam              |   3.1.0 |
| @uppy/react               |   3.0.1 | @uppy/xhr-upload          |   3.0.1 |
| @uppy/remote-sources      |   1.0.1 | uppy                      |   3.0.1 |

- @uppy/dashboard,@uppy/webcam: add nativeCameraFacingMode to Webcam and Dashboard (Artur Paikin / transloadit#4047)
- meta: upgrade to Jest 29 (Antoine du Hamel / transloadit#4049)
- @uppy/svelte: update peer dependencies (Antoine du Hamel / transloadit#4065)
- @uppy/react: useUppy: fix unmount on NextJS dev mode (Matt Jesuele / transloadit#4062)
- @uppy/vue: fix missing component in docs (Antoine du Hamel / transloadit#4063)
- @uppy/angular: fix compiler warning (Antoine du Hamel / transloadit#4064)
- meta: improve CI npm install time (Antoine du Hamel / transloadit#4058)
- meta: example: fix Angular example package name (Antoine du Hamel / transloadit#4060)
- meta: upgrade to TypeScript 4.8 (Antoine du Hamel / transloadit#4048)
- @uppy/core,@uppy/dashboard,@uppy/thumbnail-generator: update definition type files for TS 4.8 compatibility (Antoine du Hamel / transloadit#4055)
- @uppy/transloadit: improve deprecation notice (Antoine du Hamel / transloadit#4056)
- @uppy/thumbnail-generator: fix `exifr` import (Antoine du Hamel / transloadit#4054)
- @uppy/utils: fix `relativePath` when drag&dropping a folder (Antoine du Hamel / transloadit#4043)
- @uppy/companion: Fix Companion license (Merlijn Vos / transloadit#4044)
- e2e: add tests for AWS (Antoine du Hamel / transloadit#3665)
- meta: Only publish Companion to Dockerhub on release (Merlijn Vos / transloadit#4037)
- meta: fix linter warnings (Antoine du Hamel / transloadit#4039)
- @uppy/utils: Post-release website fixes (Merlijn Vos / transloadit#4038)
- @uppy/angular: fix peer dependencies (Antoine du Hamel / transloadit#4035)
- meta: uppy.io homepage: Add Tus (Artur Paikin)
- meta: Fix uppy.io homepage example (Artur Paikin)
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.

Compilation error in the Uppy type definition files with Typescript 4.8.2
3 participants