Gutenboarding: Fix DomainSuggestionsQuery type

[ockham]

Changes proposed in this Pull Request

Make DomainSuggestionsQuery extend InputArgs in order to add an index signature,
which we need for using DomainSuggestionQuery objects with addQueryArgs.

This fixes one TS error.

Also simplify a type export while we're at it.

Testing instructions

npm run typecheck -- --project client/landing/gutenboarding

should give one error less than on master.

Also, verify that the app still builds and runs.

[matticbot]

Test live: https://calypso.live/?branch=update/gutenboarding-domain-suggestions-query-type

[matticbot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

[sirreal]

I'd prefer to just get them all with #38401

When thinking about it, extending this opens our type up for the application more than is desired. The type assertion in #38401 should leave our application with better safety and completion, and we can just tell the type system the type is fine (which it is).

[sirreal]

Oh, did we need this?

[sirreal]

This should probably be exported. Want to add it in #38401?

[ockham]

It is being exported, just on a separate line. I can add the change to #38401 though.

[ockham]

I'd prefer to just get them all with #38401

When thinking about it, extending this opens our type up for the application more than is desired. The type assertion in #38401 should leave our application with better safety and completion, and we can just tell the type system the type is fine (which it is).

I honestly don't know 🤷‍♂️

Without the changes in this PR, DomainSuggestionQuery doesn't have an index signature -- I'm adding it by extending InputArgs (which is really just the index signature).

#38401 OTOH casts it to InputArgsObject, which I think amounts to claiming that queryObject fulfills the constraints of an InputArgsObject -- but is that true, if it actually doesn't have an index signature? 🤔

[sirreal]

My thinking is that the most important part is for the type system to support us in writing a correct application. I believe the approach in #38401 deliver that better.

interface I { [key: string]: string | undefined }
interface Bar { baz: string };

function foo(input: I) { }


const qux: Bar = { baz: 'foobarbazqux' }
// Nice completion, we know what the type is
const s: string = qux.baz

// This is an error, doesn't exist!
const s_ = qux.anythingAtAll

// The value contents are the same, but the type is different
const quux: I = {baz: 'foobarbazqux'}

// We don't know whether this exists or what it is because `I` is very open.
const t = quux.anythingAtAll


// Is this part helpful?
foo(qux) // Type error - we know it's fine but the type system isn't satisfied
foo(quux) // All good

I think it's preferable to have a more restricted type in use throughout the application than it is to satisfy the parameter types. That's exactly what we get with the type assertion in #38401 - we tell the type system it's fine.

[ockham]

// We don't know whether this exists or what it is because `I` is very open.
const t = quux.anythingAtAll

Hmm right, that's a helpful example, thanks. It didn't occur to me that the index signature opened the interface up for basically any field, as long as it was compatible with it. Definitely something we should tighten.

Alas, I'm still not a fan of

( queryObject as unknown ) as InputArgsObject

To me, this reads like a very powerful cast (that could be basically applied to any type), no?

 ( 123 as unknown ) as InputArgsObject

So while it expresses that we know that queryObject's type, it's kind of too powerful IMO -- feels like a last resort for a case where the type system for whatever reason believes that our object's type is totally different, when it's really quite similar.

Can we somehow express that queryObject is kind of a narrowed-down InputArgsObject (with stricter constraints)?

[sirreal]

Alas, I'm still not a fan of

( queryObject as unknown ) as InputArgsObject

To me, this reads like a very powerful cast (that could be basically applied to any type), no?

 ( 123 as unknown ) as InputArgsObject

Accurate, this is the sledge hammer of the type system. It seems like the lesser concession and only affects us in one place.

So while it expresses that we know that queryObject's type, it's kind of too powerful IMO -- feels like a last resort for a case where the type system for whatever reason believes that our object's type is totally different, when it's really quite similar.

Can we somehow express that queryObject is kind of a narrowed-down InputArgsObject (with stricter constraints)?

I don't believe this is possible, the function signature requires that type. To the best of my understanding, anything narrowed-down will, by definition, not satisfy that type.

I think this is an issue that should be addressed upstream in the type definition, or eventually right in Gutenberg, where they're adding types (WordPress/gutenberg#18838) and we're looking to generate type declarations from live code (WordPress/gutenberg#18942).

[ockham]

Okay, let's go with #38401 for now (so we can finally merge #37945), and try to maybe fix the underlying problem upstream.