Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

JoostK
Copy link
Member

@JoostK JoostK commented Oct 13, 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 #30080

@alxhub
Copy link
Member

alxhub commented Oct 24, 2019

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
@AndrewKushnir
Copy link
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

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