fix: auth typings and reduced type ambiguity

[damusix]

• RFC, claude code helped me with this one. It looks correct.
• Fixed regression in MergeType utility that broke auth Possible typing regression between v21.3.12 and v21.4.4 #4561
• Add type missed merges
• Remove ambiguous types
• Added more TS tests. LMK if you want me to remove code comments

[fjeldstad]

This should have been part of a major version bump as the type changes are breaking in some scenarios. For example when extracting a header value; that used to be string | undefined while now it is string | string[] | undefined meaning that it cannot be assigned to a string variable anymore. Even if it is more correct now, I don't think consumers expect the build to break from installing a patch version bump.

[Marsup]

@fjeldstad we already answered that in #4563. Breaking types is inherent to the typesrcipt ecosystem, nobody releases a major version just for types correctness.

[kanongil]

nobody releases a major version just for types correctness

FYI, some projects do try to do that by eg. adhering to the proposed semver-ts spec. However, even with that, changes in minor releases can cause type compilation issues! As they state (paraphrased):

We propose the rules below, with the caveat that they will not prevent all possible breakage—only the majority of it.

As such, for typescript development, one must expect that any typings update can potentially cause "breakage".

[kanongil]

@fjeldstad argued that the narrowed types should have been in a breaking release. As already discussed, breakage is somewhat expected.

I will now argue that the types should never have been narrowed, as evident from my inline comments. Maybe undo this mess?

[kanongil]

What is going on here? This is a bad choice. After validation the value can be anything. Parsing to a number or a Date are valid choices. This should probably just be unknown.

[damusix]

The thing is: If you don't validate, it will most certainly be string | string[] | undefined. This is the actual possibility of runtime data types Hapi would produce.

If you do validate, it's whatever you've set with Joi, which is a custom definition, and depends on you not using schema.strict(), which would make string | string[] | undefined true again. At that point you might as well include a custom ServerRoute if you're depending on the types to be correct:

ServerRoute<{
  Headers: { 
    'x-my-custom-header': number 
    'x-date': Date
  }
}>

unknown | unknown[] | undefined would imply: reality depends on the user's Joi usage and Joi configuration for ALL routes— Joi is the source of runtime truth. This seems like the wrong assumption.

[kanongil]

I understand your point and it definitely has merits, but ultimately I think it is flawed.

Header, query, and param validation and transformation is core to using Hapi, so it doesn't make sense to have a default type that only safely applies when it is not used. It does not have to be Joi, but some kind of validation and possible transformation is very much expected.

Whether the default should be unknown or any can be debated, but given Hapi tends to prefer safety and strictness I would probably go with unknown, so any parameter usage would have to be explicit.

Btw, for query params the types are even more uncertain, since it also depends on the parser server option. With eg. the suggested qs module it can also contain plain objects, which is now impossible to type, as it is not a subset of string | string[] | undefined.

declare module "@hapi/hapi" {
    interface ReqRefDefaults {
        Query: {
            [key: string]: string | string[] | Record<string, object> | undefined;
        };
    }
}

…fails with this error:

node_modules/@hapi/hapi/lib/types/request.d.ts:298:18 - error TS2430: Interface 'ReqRefDefaults' incorrectly extends interface 'InternalRequestDefaults'.
  Types of property 'Query' are incompatible.
    Type '{ [key: string]: string | string[] | Record<string, object> | undefined; }' is not assignable to type 'RequestQuery'.
      'string' index signatures are incompatible.
        Type 'string | string[] | Record<string, object> | undefined' is not assignable to type 'string | string[] | undefined'.
          Type 'Record<string, object>' is not assignable to type 'string | string[] | undefined'.
            Type 'Record<string, object>' is missing the following properties from type 'string[]': length, pop, push, concat, and 29 more.

[kanongil]

Again, the actual type is the result of the validation step.

[kanongil]

Again, the actual type is the result of the validation step.

[kanongil]

With the current narrow typing, it is impossible to change eg. the Query default to another type. It can only be further narrowed.