Skip to content

fix(ivy): support abstract directives in template type checking - #33131

Closed
JoostK wants to merge 1 commit into
angular:masterfrom
JoostK:ngtsc-abstract-directives
Closed

JoostK wants to merge 1 commit into
angular:masterfrom
JoostK:ngtsc-abstract-directives

Conversation

@JoostK

@JoostK JoostK commented Oct 13, 2019

Copy link
Copy Markdown
Member

Recently it was made possible to have a directive without selector,
which are referred to as abstract directives. Such directives should not
be registered in an NgModule, but can still contain decorators for
inputs, outputs, queries, etc. The information from these decorators and
the @Directive() decorator itself needs to be registered with the
central MetadataRegistry so that other areas of the compiler can
request information about a given directive, an example of which is the
template type checker that needs to know about the inputs and outputs of
directives.

Prior to this change, however, abstract directives would only register
themselves with the MetadataRegistry as being an abstract directive,
without all of its other metadata like inputs and outputs. This meant
that the template type checker was unable to resolve the inputs and
outputs of these abstract directives, therefore failing to check them
correctly. The typical error would be that some property does not exist
on a DOM element, whereas said property should have been bound to the
abstract directive's input.

This commit fixes the problem by always registering the metadata of a
directive or component with the MetadataRegistry. Tests have been
added to ensure abstract directives are handled correctly in the
template type checker, together with tests to verify the form of
abstract directives in declaration files.

Fixes #30080

@alxhub
alxhub force-pushed the ngtsc-abstract-directives branch from 52552f9 to 61c2c2f Compare October 24, 2019 17:03
@alxhub

alxhub commented Oct 24, 2019

Copy link
Copy Markdown
Member

Presubmit

@AndrewKushnir AndrewKushnir added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 24, 2019
@AndrewKushnir AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Oct 24, 2019
Recently it was made possible to have a directive without selector,
which are referred to as abstract directives. Such directives should not
be registered in an NgModule, but can still contain decorators for
inputs, outputs, queries, etc. The information from these decorators and
the `@Directive()` decorator itself needs to be registered with the
central `MetadataRegistry` so that other areas of the compiler can
request information about a given directive, an example of which is the
template type checker that needs to know about the inputs and outputs of
directives.

Prior to this change, however, abstract directives would only register
themselves with the `MetadataRegistry` as being an abstract directive,
without all of its other metadata like inputs and outputs. This meant
that the template type checker was unable to resolve the inputs and
outputs of these abstract directives, therefore failing to check them
correctly. The typical error would be that some property does not exist
on a DOM element, whereas said property should have been bound to the
abstract directive's input.

This commit fixes the problem by always registering the metadata of a
directive or component with the `MetadataRegistry`. Tests have been
added to ensure abstract directives are handled correctly in the
template type checker, together with tests to verify the form of
abstract directives in declaration files.

Fixes angular#30080
@alxhub
alxhub force-pushed the ngtsc-abstract-directives branch from 61c2c2f to 3d1960c Compare October 24, 2019 17:26
@AndrewKushnir

Copy link
Copy Markdown
Contributor

@AndrewKushnir AndrewKushnir removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Oct 24, 2019
@alxhub alxhub added the action: merge The PR is ready for merge by the caretaker label Oct 24, 2019
@angular-automatic-lock-bot

Copy link
Copy Markdown

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.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Nov 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit area: compiler Issues related to `ngc`, Angular's template compiler cla: yes effort1: hours freq2: medium risk: medium target: major This PR is targeted for the next major release type: bug/fix workaround2: non-obvious

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inputs are not inherited when Ivy is enabled

4 participants