-
Notifications
You must be signed in to change notification settings - Fork 2k
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
@uppy/core: migrate to TS #4811
Conversation
"rootDir": "./src", | ||
"resolveJsonModule": false, | ||
"noImplicitAny": false, | ||
"skipLibCheck": true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that's what causing production TS to ignore untyped dependencies, and because it's not present in tsconfig.json
, it reports an error. Maybe we want to re-enable it on prod (and have @ts-expect-error
in source files), or we'd want to skip it on the shared options
656c166
to
ba05eec
Compare
Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
this.#uppy = uppy | ||
} | ||
|
||
on<K extends keyof _UppyEventMap<M, B>>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why three on
s?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One is deprecated
Congrats on landing this team! Milestone! ✨✨💖✨ |
Massive effort by @Murderlon and @aduh95 (initial TS build setup, utils)! |
Most noteworthy is that the Uppy class now takes two generics:
This is so we can correctly type Uppy files with custom metadata and the response body from uploaders.
In the case of
xhr-upload
the body is something people have to type themselves because it's their own backend. When working with tus or Transloadit, I imagine we probably need to do something like this: