fix some types in core

[mifi]

I think we should strive for safe typings in core, because a lot of other code depends on it. this PR fixes some of the issues, and adds todos for some remaining issues with core typings.

1. don't type assert opts, but instead use default values so that typescript understands it.
   This will prevent this.opts looking like they are required when they in reality are undefined (at runtime).
   This change also uncovered the fact that locale is in fact optional, so change types to reflect the fact.
   This change also fixes the problem where any options set to undefined in the constructor would overwrite any default options, example:
   { ...{ id: 'uppy' }, ...{ id: undefined } } becomes { id: undefined }

2. remove some other unsafe type assertions

[github-actions]

Diff output files

diff --git a/packages/@uppy/core/lib/Uppy.js b/packages/@uppy/core/lib/Uppy.js
index ef5809d..cca1261 100644
--- a/packages/@uppy/core/lib/Uppy.js
+++ b/packages/@uppy/core/lib/Uppy.js
@@ -56,6 +56,17 @@ _Symbol$for = Symbol.for("uppy test: getPlugins");
 _Symbol$for2 = Symbol.for("uppy test: createUpload");
 export class Uppy {
   constructor(_opts) {
+    var _opts$id,
+      _opts$autoProceed,
+      _opts$allowMultipleUp,
+      _opts$allowMultipleUp2,
+      _opts$debug,
+      _opts$meta,
+      _opts$onBeforeFileAdd,
+      _opts$onBeforeUpload,
+      _opts$store,
+      _opts$logger,
+      _opts$infoTimeout;
     Object.defineProperty(this, _runUpload, {
       value: _runUpload2,
     });
@@ -159,29 +170,32 @@ export class Uppy {
       value: new Map(),
     });
     this.defaultLocale = locale;
-    const defaultOptions = {
-      id: "uppy",
-      autoProceed: false,
-      allowMultipleUploadBatches: true,
-      debug: false,
-      restrictions: defaultRestrictionOptions,
-      meta: {},
-      onBeforeFileAdded: (file, files) => !Object.hasOwn(files, file.id),
-      onBeforeUpload: files => files,
-      store: new DefaultStore(),
-      logger: justErrorsLogger,
-      infoTimeout: 5000,
-    };
-    const merged = {
-      ...defaultOptions,
-      ..._opts,
-    };
     this.opts = {
-      ...merged,
+      ..._opts,
+      id: (_opts$id = _opts == null ? void 0 : _opts.id) != null ? _opts$id : "uppy",
+      autoProceed: (_opts$autoProceed = _opts == null ? void 0 : _opts.autoProceed) != null ? _opts$autoProceed : false,
+      allowMultipleUploadBatches:
+        (_opts$allowMultipleUp = _opts == null ? void 0 : _opts.allowMultipleUploadBatches) != null
+          ? _opts$allowMultipleUp
+          : true,
+      allowMultipleUploads: (_opts$allowMultipleUp2 = _opts == null ? void 0 : _opts.allowMultipleUploads) != null
+        ? _opts$allowMultipleUp2
+        : true,
+      debug: (_opts$debug = _opts == null ? void 0 : _opts.debug) != null ? _opts$debug : false,
       restrictions: {
-        ...defaultOptions.restrictions,
-        ...(_opts && _opts.restrictions),
+        ...defaultRestrictionOptions,
+        ...(_opts == null ? void 0 : _opts.restrictions),
       },
+      meta: (_opts$meta = _opts == null ? void 0 : _opts.meta) != null ? _opts$meta : {},
+      onBeforeFileAdded: (_opts$onBeforeFileAdd = _opts == null ? void 0 : _opts.onBeforeFileAdded) != null
+        ? _opts$onBeforeFileAdd
+        : (file, files) => !Object.hasOwn(files, file.id),
+      onBeforeUpload: (_opts$onBeforeUpload = _opts == null ? void 0 : _opts.onBeforeUpload) != null
+        ? _opts$onBeforeUpload
+        : files => files,
+      store: (_opts$store = _opts == null ? void 0 : _opts.store) != null ? _opts$store : new DefaultStore(),
+      logger: (_opts$logger = _opts == null ? void 0 : _opts.logger) != null ? _opts$logger : justErrorsLogger,
+      infoTimeout: (_opts$infoTimeout = _opts == null ? void 0 : _opts.infoTimeout) != null ? _opts$infoTimeout : 5000,
     };
     if (_opts && _opts.logger && _opts.debug) {
       this.log(
@@ -745,7 +759,8 @@ export class Uppy {
     if (sizedFiles.length === 0) {
       const progressMax = inProgress.length * 100;
       const currentProgress = unsizedFiles.reduce((acc, file) => {
-        return acc + file.progress.percentage;
+        var _file$progress$percen;
+        return acc + ((_file$progress$percen = file.progress.percentage) != null ? _file$progress$percen : 0);
       }, 0);
       const totalProgress = Math.round(currentProgress / progressMax * 100);
       this.setState({
@@ -761,7 +776,7 @@ export class Uppy {
     totalSize += averageSize * unsizedFiles.length;
     let uploadedSize = 0;
     sizedFiles.forEach(file => {
-      uploadedSize += file.progress.bytesUploaded;
+      uploadedSize += Number(file.progress.bytesUploaded);
     });
     unsizedFiles.forEach(file => {
       uploadedSize += averageSize * (file.progress.percentage || 0) / 100;

[Murderlon]

I think this is incorrect and whether it's Partial or not is up to the implementer of the generic.

[mifi]

Hm, so state is supposed to have all metadata from M (e.g. not partial)? then we need to either:

• change UppyOptions so that it also takes in meta: M instead of meta: Partial<M>
• or change this line

  uppy/packages/@uppy/core/src/Uppy.ts

  Line 388 in e4daea9

  meta: { ...this.opts.meta },

  so that it somehow sets default values for all M keys, but I don't know how to do that because M is not known

[Murderlon]

so state is supposed to have all metadata from M

Yes that's correct

[mifi]

i've now tried to update the meta types to M instead of Partial<M> but TS is failing because we are trying to assign this.opts.meta to Partial<M> ({}) here:

uppy/packages/@uppy/core/src/Uppy.ts

Line 352 in e4daea9

meta: opts?.meta ?? {},

I think it would be a breaking change to alter this behavior

[Murderlon]

We can do this as a type meta: M | undefined. Which is different from making all properties optional.

[mifi]

I don't think that would help because {} is not assignable to M | undefined. or do you mean changing the runtime behavior to:

meta: opts?.meta

[mifi]

if i do that i get

M could be instantiated with an arbitrary type which could be unrelated to M | undefined .

[Murderlon]

Are you sure about this? defaultLocale is indeed OptionalPluralizeLocale but I don't think this is.

[mifi]

we're getting typescript errors if it's set to Locale

[Murderlon]

That might be but this option should be Locale.