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

TS strict mode #5258

Merged
merged 7 commits into from
Jun 20, 2024
Merged

TS strict mode #5258

merged 7 commits into from
Jun 20, 2024

Conversation

Murderlon
Copy link
Member

@Murderlon Murderlon commented Jun 18, 2024

Ref: https://github.com/transloadit/uppy-planning/issues/36#issuecomment-2173650732
Blocked on #5050 (TS errors in provider-views which I'm not fixing to prevent merge conflicts)

  • Apply feedback
  • Changes for strict mode
  • useUnknownInCatchVariables to false as we have many errors from this but there isn't a good way at the moment to type this. Uppy puts properties on errors all over the place, often without proper error classes. So settings this to true would mean hard casting everything with any so might as well let TS set the default to any. Even if we turn it on, the only way to narrow the type is with instanceof which is a check we generally avoid.

We have to decide what to do with allowImportingTsExtensions. I've left it there too because we have many ts imports, particularly in tests. But Remco (strongly) recommended to remove it.

@Murderlon Murderlon requested review from mifi and aduh95 June 18, 2024 09:37
@Murderlon Murderlon self-assigned this Jun 18, 2024
Copy link
Contributor

github-actions bot commented Jun 18, 2024

Diff output files
diff --git a/packages/@uppy/aws-s3/lib/MultipartUploader.js b/packages/@uppy/aws-s3/lib/MultipartUploader.js
index ff1f21c..e104fdc 100644
--- a/packages/@uppy/aws-s3/lib/MultipartUploader.js
+++ b/packages/@uppy/aws-s3/lib/MultipartUploader.js
@@ -69,11 +69,11 @@ class MultipartUploader {
     });
     Object.defineProperty(this, _chunks, {
       writable: true,
-      value: void 0,
+      value: [],
     });
     Object.defineProperty(this, _chunkState, {
       writable: true,
-      value: void 0,
+      value: [],
     });
     Object.defineProperty(this, _data, {
       writable: true,
diff --git a/packages/@uppy/aws-s3/lib/index.js b/packages/@uppy/aws-s3/lib/index.js
index def9666..2171565 100644
--- a/packages/@uppy/aws-s3/lib/index.js
+++ b/packages/@uppy/aws-s3/lib/index.js
@@ -640,7 +640,7 @@ function _uploadLocalFile2(file) {
       log: function() {
         return _this.uppy.log(...arguments);
       },
-      getChunkSize: this.opts.getChunkSize ? this.opts.getChunkSize.bind(this) : null,
+      getChunkSize: this.opts.getChunkSize ? this.opts.getChunkSize.bind(this) : undefined,
       onProgress,
       onError,
       onSuccess,
diff --git a/packages/@uppy/file-input/lib/FileInput.js b/packages/@uppy/file-input/lib/FileInput.js
index c318ddf..b52a1c8 100644
--- a/packages/@uppy/file-input/lib/FileInput.js
+++ b/packages/@uppy/file-input/lib/FileInput.js
@@ -15,6 +15,7 @@ export default class FileInput extends UIPlugin {
       ...defaultOptions,
       ...opts,
     });
+    this.input = null;
     this.id = this.opts.id || "FileInput";
     this.title = "File Input";
     this.type = "acquirer";
diff --git a/packages/@uppy/provider-views/lib/View.js b/packages/@uppy/provider-views/lib/View.js
index 37db53a..01d961f 100644
--- a/packages/@uppy/provider-views/lib/View.js
+++ b/packages/@uppy/provider-views/lib/View.js
@@ -1,6 +1,7 @@
 import remoteFileObjToLocal from "@uppy/utils/lib/remoteFileObjToLocal";
 export default class View {
   constructor(plugin, opts) {
+    this.isShiftKeyPressed = false;
     this.filterItems = items => {
       const state = this.plugin.getPluginState();
       if (!state.filterInput || state.filterInput === "") {
diff --git a/packages/@uppy/screen-capture/lib/RecorderScreen.js b/packages/@uppy/screen-capture/lib/RecorderScreen.js
index 52e5342..ea999a6 100644
--- a/packages/@uppy/screen-capture/lib/RecorderScreen.js
+++ b/packages/@uppy/screen-capture/lib/RecorderScreen.js
@@ -14,6 +14,10 @@ import StopWatch from "./StopWatch.js";
 import StreamStatus from "./StreamStatus.js";
 import SubmitButton from "./SubmitButton.js";
 class RecorderScreen extends Component {
+  constructor() {
+    super(...arguments);
+    this.videoElement = null;
+  }
   componentWillUnmount() {
     const {
       onStop,
diff --git a/packages/@uppy/screen-capture/lib/ScreenCapture.js b/packages/@uppy/screen-capture/lib/ScreenCapture.js
index 0ccc269..009813e 100644
--- a/packages/@uppy/screen-capture/lib/ScreenCapture.js
+++ b/packages/@uppy/screen-capture/lib/ScreenCapture.js
@@ -49,6 +49,12 @@ export default class ScreenCapture extends UIPlugin {
       ...defaultOptions,
       ...opts,
     });
+    this.videoStream = null;
+    this.audioStream = null;
+    this.userDenied = false;
+    this.recorder = null;
+    this.outputStream = null;
+    this.recordingChunks = null;
     this.mediaDevices = getMediaDevices();
     this.protocol = location.protocol === "https:" ? "https" : "http";
     this.id = this.opts.id || "ScreenCapture";
diff --git a/packages/@uppy/screen-capture/lib/StopWatch.js b/packages/@uppy/screen-capture/lib/StopWatch.js
index bf7415e..6e106d5 100644
--- a/packages/@uppy/screen-capture/lib/StopWatch.js
+++ b/packages/@uppy/screen-capture/lib/StopWatch.js
@@ -39,6 +39,7 @@ class StopWatch extends Component {
       fontSize: "3rem",
       fontFamily: "Courier New",
     };
+    this.timerRunning = false;
     this.state = {
       elapsedTime: 0,
     };
diff --git a/packages/@uppy/transloadit/lib/Assembly.js b/packages/@uppy/transloadit/lib/Assembly.js
index f9b896c..ab6526f 100644
--- a/packages/@uppy/transloadit/lib/Assembly.js
+++ b/packages/@uppy/transloadit/lib/Assembly.js
@@ -62,7 +62,7 @@ class TransloaditAssembly extends Emitter {
     });
     Object.defineProperty(this, _sse, {
       writable: true,
-      value: void 0,
+      value: null,
     });
     this.status = assembly;
     this.pollInterval = null;
diff --git a/packages/@uppy/transloadit/lib/index.js b/packages/@uppy/transloadit/lib/index.js
index 77c1526..75d3a12 100644
--- a/packages/@uppy/transloadit/lib/index.js
+++ b/packages/@uppy/transloadit/lib/index.js
@@ -107,6 +107,7 @@ export default class Transloadit extends BasePlugin {
       writable: true,
       value: void 0,
     });
+    this.restored = null;
     Object.defineProperty(this, _onFileUploadURLAvailable, {
       writable: true,
       value: rawFile => {
diff --git a/packages/@uppy/webcam/lib/Webcam.js b/packages/@uppy/webcam/lib/Webcam.js
index 8b0a6c6..5ca7a7f 100644
--- a/packages/@uppy/webcam/lib/Webcam.js
+++ b/packages/@uppy/webcam/lib/Webcam.js
@@ -72,6 +72,10 @@ export default class Webcam extends UIPlugin {
       writable: true,
       value: void 0,
     });
+    this.stream = null;
+    this.recorder = null;
+    this.recordingChunks = null;
+    this.captureInProgress = false;
     this.mediaDevices = getMediaDevices();
     this.supportsUserMedia = !!this.mediaDevices;
     this.protocol = location.protocol.match(/https/i) ? "https" : "http";

@aduh95
Copy link
Contributor

aduh95 commented Jun 20, 2024

There are some linter failure to address.

Do you want me to implement my proposed feedback (to use ?: instead of | undefined)? I'm happy to do it if that's simpler for you.

  • the only way to narrow the type is with instanceof which is a check we generally avoid.

It's not the only way, I think the preferred solution we settled on was to use function isThatErrorType(err: unknown): err is ThatErrorType util functions. But in any case, your argument still applies, it's much simpler to set useUnknownInCatchVariables to false.

@Murderlon
Copy link
Member Author

Murderlon commented Jun 20, 2024

There are some linter failure to address.

As mentioned I'm waiting for #5050 to land to avoid conflicts in provider-views, which is where CI is failing. But now that I think of it, suppose I can also fix it so we can merge this, since strict mode probably reveals more errors in that PR which should be fixed there.

Murderlon and others added 4 commits June 20, 2024 09:41
Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
* 4.x:
  @uppy/companion: fix merge conflicts
  Release: uppy@3.27.0 (#5257)
  Bump ws from 8.8.1 to 8.17.1 (#5256)
  @uppy/google-photos: add plugin (#5061)
  updating aws-nodejs example listParts logic for resuming uploads (#5192)
  Bump docker/login-action from 3.1.0 to 3.2.0 (#5217)
  Bump docker/build-push-action from 5.3.0 to 5.4.0 (#5252)
Co-authored-by: Antoine du Hamel <antoine@transloadit.com>
@Murderlon Murderlon merged commit 88d508f into 4.x Jun 20, 2024
17 checks passed
@Murderlon Murderlon deleted the ts-strict branch June 20, 2024 14:54
github-actions bot added a commit that referenced this pull request Jun 27, 2024
| Package                |       Version | Package                |       Version |
| ---------------------- | ------------- | ---------------------- | ------------- |
| @uppy/audio            |  2.0.0-beta.7 | @uppy/image-editor     |  3.0.0-beta.6 |
| @uppy/aws-s3           |  4.0.0-beta.8 | @uppy/instagram        |  4.0.0-beta.7 |
| @uppy/box              |  3.0.0-beta.8 | @uppy/onedrive         |  4.0.0-beta.8 |
| @uppy/companion        | 5.0.0-beta.11 | @uppy/provider-views   | 4.0.0-beta.10 |
| @uppy/companion-client |  4.0.0-beta.8 | @uppy/react            |  4.0.0-beta.8 |
| @uppy/core             | 4.0.0-beta.11 | @uppy/screen-capture   |  4.0.0-beta.6 |
| @uppy/dashboard        | 4.0.0-beta.11 | @uppy/transloadit      | 4.0.0-beta.10 |
| @uppy/drop-target      |  3.0.0-beta.6 | @uppy/unsplash         |  4.0.0-beta.8 |
| @uppy/dropbox          |  4.0.0-beta.9 | @uppy/url              |  4.0.0-beta.8 |
| @uppy/facebook         |  4.0.0-beta.7 | @uppy/utils            |  6.0.0-beta.9 |
| @uppy/file-input       |  4.0.0-beta.6 | @uppy/vue              |  2.0.0-beta.4 |
| @uppy/form             |  4.0.0-beta.5 | @uppy/webcam           |  4.0.0-beta.9 |
| @uppy/golden-retriever |  4.0.0-beta.6 | @uppy/xhr-upload       |  4.0.0-beta.7 |
| @uppy/google-drive     |  4.0.0-beta.1 | @uppy/zoom             |  3.0.0-beta.7 |
| @uppy/google-photos    |  0.2.0-beta.2 | uppy                   | 4.0.0-beta.13 |

- @uppy/companion: implement facebook app secret proof (Mikael Finstad / #5249)
- @uppy/provider-views: `Loader.tsx` - delete the file (Evgenia Karunus / #5284)
- @uppy/vue: fix passing of `props` (Antoine du Hamel / #5281)
- @uppy/google-photos: fix various issues (Mikael Finstad / #5275)
- @uppy/transloadit: fix strict type errors (Antoine du Hamel / #5271)
- @uppy/transloadit: simplify plugin to always run a single assembly (Merlijn Vos / #5158)
- meta: update Yarn version and npm deps (Antoine du Hamel / #5269)
- docs: prettier: 3.2.5 -> 3.3.2 (Antoine du Hamel / #5270)
- @uppy/provider-views: Provider views rewrite (.files, .folders => .partialTree) (Evgenia Karunus / #5050)
- @uppy/react: TS strict mode (Merlijn Vos / #5258)
- meta: simplify `build:ts` script (Antoine du Hamel / #5262)
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