meta: validate defaultOptions for stricter option types

[aduh95]

1. Removes [key: string]: unknown from PluginOptions – that was probably added by mistake
2. Ensures the AlwaysDefinedKeys are one of the optional key of the Opts type. That should help catch instances where we defined by mistake a default value for a mandatory option, or we mistakenly mark as required an option meant to be optional.
3. Fix the resulting type errors in the codebase and tests.

[github-actions]

Diff output files

diff --git a/packages/@uppy/xhr-upload/lib/index.js b/packages/@uppy/xhr-upload/lib/index.js
index a864f43..bb39afe 100644
--- a/packages/@uppy/xhr-upload/lib/index.js
+++ b/packages/@uppy/xhr-upload/lib/index.js
@@ -41,7 +41,6 @@ function setTypeInBlob(file) {
   return dataWithUpdatedType;
 }
 const defaultOptions = {
-  endpoint: "",
   formData: true,
   fieldName: "file",
   method: "post",

[Murderlon]

Can't we in this case remove the Partial's above so this type can stay simple? This seems a bit intense.

[aduh95]

The issue is that we pass keyof typeof default…, so there's no way to validate the values of the object, only the keys. If there was a way to pass typeof default… instead, we would no longer need the satisfies above. I tried the following which didn't work (Type 'keyof DefaultValues' does not satisfy the constraint 'keyof Opts'.):

export type DefinePluginOpts<
  Opts,
  DefaultValues extends OnlyOptionals<Opts>,
> = Opts & Required<Pick<Opts, keyof DefaultValues>>

[Murderlon]

So you're saying this is only a problem because we have nested object types here with actions and such? The problem of "no way to validate the values of the object" is not present elsewhere?

Also, we are exporting the wrong type. ImageEditorOpts should be called Opts and not be exported and Opts should be called ImageEditorOpts and exported. We want to export the user facing type, not the internal type I think.

[aduh95]

ImageEditorOpts should […] not be exported

Good catch, I've renamed it InternalImageEditorOpts.

So you're saying this is only a problem because we have nested object types here with actions and such? The problem of "no way to validate the values of the object" is not present elsewhere?

That's not what I'm saying, although having nested objects certainly makes the types more complicated.
Currently we need the satisfies to ensure the default values match the types of the associated option. Maybe there's a way to validate that somewhere else, in a more transparent way, but I wasn't able to find a way to do that.

[Murderlon]

I checked it out locally and and removed the Partial's and got no errors. Returning to the simple type of InternalImageEditorOpts also gave no errors. Where do we see the effect of this more complex type then?

[aduh95]

Well of course you won't see any error if you have looser types 🤷‍♂️ the idea is to have TS report when we use an incompatible type (i.e. you should test by changing the default value to something incompatible, e.g.
)

[aduh95]

I misread your comment, I thought you removed the satisfies Partial<…>, but I guess what you meant is you replaced it with satisfies …; sorry for the confusion, my previous comment only addressed your second point.

You don't see any error because there are no required types in that options. We could indeed use the simpler type (i.e. without Partial<>), but:

• if we ever end up adding a required option, we would get annoying TS error and need to go back and to wrap in Partial<>.
• because we have required options in some plugins, it's more consistent to use Partial<> everywhere.

There might be a TS wizard out there that would be able to help us hide those satisfies into the DefinePluginOpts utility type, but I'm afraid that's out of my league.

[Murderlon]

nit: the exported type should be named ImageEditorOpts to be consistent with other plugin type export naming

There is also a build error. Then ready to go.