Type Constructors for Generic Directives

[zarend]

Background Knowledge Required

This requires a bit of background knowledge

• How to use bound generics in typescript
• How to create a Directive in angular
• type-check-block (tcb) generation
  -- type constructors
  -- TcbOps
  -- inlining, specifically of type constructors

Motivation

This PR came from an edge case with generic directives – specifically directives with bound generic parameters.

Creating a generic directive with a bound parameters creates an edge case for TCB generation. Let's see the behavior in VSCode for this situation.

In this screenshot, we have a two confusing error messages.

1. ng serve reports that we cannot bind to notAnInput because that property does not exist. This error is expected, but it doesn't show up in the IDE.

2. the language service threw an exception with This component requires inline type checking which is not available in this environment.

The type generation code cannot inline the type constructor for this component, which causes it to throw an exception before finishing type checking the template. This means that part of the template will not be typechecked and VSCode will not be able to present diagnostics in the skipped sections.

Why bound generics are a "no-go" for compiling TCBs

Bound generics can reference private members which are impossible for the compiler to import into the tcb. They can also contain other things such as symbols which would be impossible to import.

Application Code

interface PrivateInterface{}

@Componet({ selector: 'bad' })
export class BadComponent <T extends PrivateInterface>{}

TCB bad.component.typecheck.ts

  var ctor1: ()<T extends PrivateInterface> => BadComponent<T> = () => (null!);
  // but wait, how do we import PrivateInterface?

The solutions approach is to assign any to generic arguments for constructors with bound parameters.

var ctor1: BadComponent<any> = (null!);

Why inline type constructors impact performance.

When we compile a program that requires inlining type constructors, we change add generated code to the component file that the user originally wrote.

bad.component.ts

...
export class BadComponent <T extends PrivateInterface>{
...
// type constructor will look something like this, but not exactly like this
ngTypeConstructor: ()<T> => BadComponent<T> = () => (null!) 
}

Changing this file can trigger a second compile, which increases the total compilation time.

Also, we cannot change a user's file while it's been edited, which is why inlining is not available for the language service. It's only available when we compile a build target for the program.

Solution

This is a 2-part PR.

The first commit refactors our tests, which allows use to use TDD in the next commit, which fixes the bug.

Part 1: refactor tests

For the tests in //packages/compiler-cli/src/ngtsc/typecheck, this
commits uses a TypeCheckFile for the environment, rather than a
FakeEnvironment. Using a real environment gives us more flexibility
with testing.

Part 2: implement fix for generic directives with bound parameters

This commit fixes the behavior when creating a type constructor for a directive when the following conditions are met.

1. The directive has bound generic parameters.
2. Inlining is not available. (This happens for language service compiles).

Previously, we would throw an error saying 'Inlining is not supported in this environment.' The compiler would stop type checking, and the developer could lose out on getting errors after the compiler gives up.

This commit adds a useInlineTypeConstructors to the type check config. When set to false, we use any type for bound generic parameters to avoid crashing. When set to true, we inline the type constructor when inlining is required.

[atscott]

You have a pretty good overview of the issue in the comment. Please add a similar description to the commit message. The commit messages should contain all the same information about why a change is being made - it shouldn't only be in the PR comment.

[alxhub]

All of our TCB operations have names of the form Tcb*Op, and so should this one.

I'm thinking TcbGenericDirectiveTypeWithAnyParamsOp (it's kind of unfortunate that "any" has a confusing meaning in a short name like this).

We could also rename TcbDirectiveTypeOp to TcbNonGenericDirectiveTypeOp to more cleanly separate the two.

[alxhub]

This should definitely not be false. Inline type constructors are required for how the compiler normally operates.

This should probably be set based on whether the current TypeCheckingProgramStrategy supports inline operations (it exposes a supportsInlineOperations property to report this).

[zarend]

🤦 , that is correct.

For strict mode, I made it
useInlineTypeConstructors: this.typeCheckingProgramStrategy.supportsInlineOperations,
That's because inlining gives us strict type checking with bound generics because we do not have to set the generic params to any.

For non-strict mode, I set it to false. That's because we want to be as lenient as possible in non-strict mode. When useInlineTypeConstructors is false, we use any for the generic params of bound generics instead of trying to infer the types. It's better to avoid inlining unless we have to because it's problematic ( perf issues, complications with editing user's files).

[alxhub]

Should we check !this.dir.isGeneric too?

[mhevery]

@zarend This had a merge conflict on patch branch. Please create a new PR and label accordingly

[zarend]

@zarend This had a merge conflict on patch branch. Please create a new PR and label accordingly

Thanks, here's a PR to merge it into patch branch.
#41268

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}