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

@uppy/core: fix TypeScript errors #5593

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

EricImpact
Copy link

@ts-expect-error directive is not needed; nothing wrong with defining tus as undefined.

Do some type narrowing of err in catch block to confirm it's an Error before returning message.

Closes #5592

`@ts-expect-error` directive is not needed; nothing wrong with defining `tus` as `undefined`. 

Do some type narrowing of `err` in catch block to confirm it's an `Error` before returning message.
Copy link
Contributor

Diff output files
diff --git a/packages/@uppy/core/lib/Uppy.js b/packages/@uppy/core/lib/Uppy.js
index b4c89fe..05a12aa 100644
--- a/packages/@uppy/core/lib/Uppy.js
+++ b/packages/@uppy/core/lib/Uppy.js
@@ -520,7 +520,9 @@ export class Uppy {
     try {
       _classPrivateFieldLooseBase(this, _restricter)[_restricter].validateSingleFile(file);
     } catch (err) {
-      return err.message;
+      if (err instanceof Error) {
+        return err.message;
+      }
     }
     return null;
   }
@@ -529,7 +531,9 @@ export class Uppy {
     try {
       _classPrivateFieldLooseBase(this, _restricter)[_restricter].validateAggregateRestrictions(existingFiles, files);
     } catch (err) {
-      return err.message;
+      if (err instanceof Error) {
+        return err.message;
+      }
     }
     return null;
   }

@Murderlon Murderlon changed the title Fix TypeScript errors @uppy/core: fix TypeScript errors Jan 11, 2025
@Murderlon Murderlon added the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label Jan 11, 2025
@github-actions github-actions bot removed the safe to test Add this label on trustworthy PRs to spawn the e2e test suite label Jan 11, 2025
@Murderlon
Copy link
Member

Run corepack yarn build
Error: packages/@uppy/core/src/Uppy.ts(652,9): error TS2353: Object literal may only specify known properties, and 'tus' does not exist in type 'UppyFile<M, B>'.

Seems like it's needed afterall

Reinstate ts-expect-error flag, but this time directly above line with expected error
@EricImpact
Copy link
Author

Run corepack yarn build
Error: packages/@uppy/core/src/Uppy.ts(652,9): error TS2353: Object literal may only specify known properties, and 'tus' does not exist in type 'UppyFile<M, B>'.

Seems like it's needed afterall

I've updated the file to reinstate the @ts-expect-error flag, but placed it directly above the tus line. I couldn't test locally as I had issues trying to run npm install on the repo package, but hopefully this resolves the problem.

@Murderlon
Copy link
Member

I wonder how much this PR will help because we use "useUnknownInCatchVariables": false everywhere, so there will be many, many instances where we don't do an instanceof check on the error. Only changing this doesn't seem to improve much.

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

Successfully merging this pull request may close these issues.

Incorrect @ts-expect-error comment causes TypeScript error
2 participants