No self or forward references in type parameter defaults

[ahejlsberg]

This PR makes it an error for type parameter defaults to reference anything but previously declared type parameters (i.e. type parameters declared to the left). This is analogous to JavaScript's restriction that parameter defaults can only reference parameters to the left, and it makes nonsensical constructs such as the following become errors:

type Foo<T = T> = T[];
type Bar<T = U, U = T> = [T, U];

This is technically a breaking change, but the only known real world case that is affected is #28388. The particular use in that example makes no sense (a type parameter referencing itself in its default) and we should instead fix the .d.ts file on Definitely Typed.

Fixes #28873. Supersedes #28971 and #29150.

[weswigham]

This doesn't make much sense to me - type parameters are pretty much an a bag of inference locations - their "order" only matters for passing corresponding type arguments. That's why named type arguments are such a simple addition. How does a rule like this make sense in the context of named type arguments, where ordering isn't supposed to exist at all? Type parameter declarations don't have block scoped semantics, there is not a runtime issue with a forward reference (there's no runtime), and we already know this breaks people since this breaks the people who led to this being reported. And I'm like 90% sure this was already discussed when defaults were added, with the end result being allowing self referential defaults as while you can construct a "meaningless" default, you can also construct a recursive structure, which is still a valid type and useful as a constraint or default if handled correctly. The only recent change here is that I made the "meaningless" case depend on the constraint of the type parameter to better constrain some recursive scenarios (which is, ofc, also is broken by this as the tests bear out).

[rbuckton]

Type parameter defaults didn't originally support forward references, which is called out specifically in #13487. I'm not certain at what point this restriction was broken:

• When the checker applies defaults for type parameters, they are applied from left to right:
  • If the default for a type parameter refers to a type parameter that occurs earlier in the list, the type argument for that parameter will be used.
  • If the default for a type parameter refers to a type parameter that occurs later in the list, the empty object type {} will be used.

[weswigham]

@rbuckton that comment seems to indicate that they explicitly are supported (with the caveat that forward refs get {} rather than the default). Which is what we do, except now rather than {} we return the actual constraint (since {} for a constrained type parameter produced incorrect results).

[rbuckton]

I'm in favor of reverting the changes that introduced circularity and more clearly treating this an error, as a type parameter default having a reference a later type parameter was explicitly disallowed when the feature was originally introduced for these exact reasons.

[rbuckton]

This is effectively going back to the behavior in the original PR. It looks like the circularity was introduced in #28222 and #28423. Though, prior to this we were just defaulting to {} in TypeScript (any in JavaScript).

[rbuckton]

@weswigham: Except we defaulted to {} explicitly to avoid this exact case where the actual constraint results in a circular reference. Using {} was a silent error, similar to other cases where we silently use {} when inference fails.

[weswigham]

Using {} was a silent error, similar to other cases where we silently use {} when inference fails.

Except we don't silently use {} when inference fails - we silently use the constraint, which is usually {}. My change brought defaults in line with that. Without doing that you'd have defaults which didn't satisfy their constraints!

[rbuckton]

@weswigham: Regardless, using a default of the error type (i.e. any) and reporting an error strengthens the original design and also solves the issue that the changes in #28222 were intended to solve.

[ahejlsberg]

@weswigham The key objective with this PR is to detect and disallow circular references in defaults. They're meaningless and confusing and we already detect and disallow circular constraints for the same reason. A secondary objective is to disallow references in defaults to type parameters for which type arguments are known to have been omitted, since those are just a complicated way of referencing other defaults.

The rule that allows references only to previously declared type parameters accomplishes both. None of the previously suggested solutions accomplish either. Instead we silently revert to {} or the type parameter constraint which really is never what the author intended (after all, if it was the intent, then why the extra indirection).

If we end up implementing named type arguments (and that's a big if given the fate of #23696) we can revise these rules accordingly, but as things stand now I think this PR is the right solution.

[InExtremaRes]

@ahejlsberg Is this going to disallow this type construction?

type Constrained<T extends { [P in keyof T]: string }> = T;

I have some code like that and it seems to be out there (example, example).

EDIT: I didn't express myself correctly. I have tested the code below and it seems still works, but I wonder if that construction is valid intentionally or it is some kind of circular reference that should be avoided.

[xiegeo]

Did this broke Vuex? championswimmer/vuex-module-decorators#90

ERROR in .../node_modules/vuex-module-decorators/dist/types/vuexmodule.d.ts
2:46 Type parameter defaults can only reference previously declared type parameters.
    1 | import { ActionTree, GetterTree, Module as Mod, ModuleTree, MutationTree, Store, ActionContext } from 'vuex';
  > 2 | export declare class VuexModule<S = ThisType<S>, R = any> implements Mod<S, R> {
      |                                              ^
    3 |     static namespaced?: boolean;
    4 |     static state?: any | (() => any);
    5 |     static getters?: GetterTree<any, any>;

[mcastilho]

@xiegeo Yes. Happening to me now.