fix: annotate a couple base classes with @Directive for Ivy #17022
Conversation
googlebot
added
the
cla: yes
src/cdk/portal/portal.ts
Outdated
| // In Angular 9.x, `@Directive()` without any selector is legal (and `BasePortalModule` is not | ||
| // necessary either). | ||
| // TODO(alxhub): convert to a selectorless Directive when the CDK upgrades to Angular 9. | ||
| selector: 'abstract-base-portal-outlet', |
There was a problem hiding this comment.
Could you augment this with "do-not-use" as a prefix?
(similar below)
src/cdk/portal/portal.ts
Outdated
| @NgModule({ | ||
| declarations: [BasePortalOutlet as any], | ||
| }) | ||
| export class BasePortalOutletModule { |
There was a problem hiding this comment.
DoNotUseBasePortalOutletModule
(similar below)
Along the same lines as angular#17022. Adds annotations to the various base classes that declare inputs, because the Ivy template type checker doesn't handle base defs at the moment. Fixes angular#17121.
There was a problem hiding this comment.
Is this actually planned to be a breaking change to most libraries or are there plans to solve this prior to Ivy GA?
Update: Nevermind, it does look like this will be a breaking change for all libraries out there. It will be worst for libraries like Angular Material that aren't built using the Angular CLI and can't use the ng update migrations. Official migration guide: https://angular.io/guide/migration-undecorated-classes
Splaktar
added
P2
There are two base classes in Material/CDK which are intended to be extended by user code: BasePortalHost and MatFormFieldControl. This commit annotates them with `@Directive`. The best way to do this is to use `@Directive()` but this is not supported by Angular 8.x, on which Material and the CDK still depend. Therefore, a workaround is used with a selector which will never be matched. Modules are also added to satisfy the View Engine compiler.
jelbourn
force-pushed
the
ivy-compat-base-dirs
branch
from
e1dc7e6 to
2ccca29
Compare
|
@alxhub I pushed a new commit to this PR with the names updated and rebased, but there's a legitimate failure.
|
jelbourn
added
P1
|
I'm actually wondering on why we need to decorate these classes with It doesn't match my understanding of which classes need to be migrated according to our migrations /and migration guides. Can you help explaining this @alxhub? |
This is a resubmit of angular#17022, excluding the portal changes. It marks the `MatFormFieldControl` as a directive because it's being extended by other directives which can cause errors under Ivy.
This is a resubmit of angular#17022, excluding the portal changes. It marks the `MatFormFieldControl` as a directive because it's being extended by other directives which can cause errors under Ivy.
This is a resubmit of #17022, excluding the portal changes. It marks the `MatFormFieldControl` as a directive because it's being extended by other directives which can cause errors under Ivy.
This is a resubmit of #17022, excluding the portal changes. It marks the `MatFormFieldControl` as a directive because it's being extended by other directives which can cause errors under Ivy.
|
closing in favor of #17457 |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
There are two base classes in Material/CDK which are intended to be extended
by user code: BasePortalHost and MatFormFieldControl.
This commit annotates them with
@Directive. The best way to do this is touse
@Directive()but this is not supported by Angular 8.x, on whichMaterial and the CDK still depend. Therefore, a workaround is used with a
selector which will never be matched. Modules are also added to satisfy the
View Engine compiler.