Skip to content

Conversation

devversion
Copy link
Member

@devversion devversion commented Oct 3, 2019

Some base classes define class members with Angular features. Therefore
they need to be decorated with @directive() so that the classes can be
extended in Ivy.

This is an official migration that will be performed for CLI
applications in version 9. angular/angular#32590.

Most of these classes do not cause issues yet because the removal of ngBaseDef
has not happened yet, but it is planned.. hence the actual migration in @angular/core.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 3, 2019
@devversion devversion added blocked This issue is blocked by some external factor, such as a prerequisite PR and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Oct 3, 2019
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 3, 2019
@googlebot

This comment has been minimized.

@devversion devversion added this to the 9.0.0 milestone Oct 3, 2019
@devversion devversion added the target: major This PR is targeted for the next major release label Oct 3, 2019
@devversion devversion force-pushed the build/lint-rule-undecorated-classes-with-ng-fields branch 4 times, most recently from 98a78e4 to 868c24b Compare October 3, 2019 13:19
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@devversion devversion force-pushed the build/lint-rule-undecorated-classes-with-ng-fields branch from 868c24b to b9944b5 Compare October 3, 2019 16:30
@devversion devversion added the P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful label Oct 3, 2019
These base classes define class members with Angular features. Therefore
they need to be decorated with `@Directive()` so that the classes can be
extended in Ivy.

This is an official migration that will be performed for CLI
applications in version 9. angular/angular#32590.
@devversion devversion force-pushed the build/lint-rule-undecorated-classes-with-ng-fields branch from b9944b5 to 0832020 Compare October 3, 2019 18:49
@devversion devversion removed the blocked This issue is blocked by some external factor, such as a prerequisite PR label Oct 3, 2019
@devversion devversion changed the title build: add lint rule to ensure classes with angular features are decorated refactor: annotate remaining base classes with @directive for ivy. Oct 3, 2019
@devversion devversion changed the title refactor: annotate remaining base classes with @directive for ivy. refactor: annotate remaining base classes with @directive for ivy Oct 3, 2019
Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Oct 3, 2019
Copy link
Contributor

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mmalerba mmalerba merged commit b228f07 into angular:master Oct 4, 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 4, 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 cla: yes PR author has agreed to Google's Contributor License Agreement P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants