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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

ivy - Async pipe gets wrong ChangeDetectorRef instance when using pipe inside a component input #33677

Closed
alexzuza opened this issue Nov 8, 2019 · 0 comments

Comments

@alexzuza
Copy link
Contributor

@alexzuza alexzuza commented Nov 8, 2019

馃悶 bug report

Affected Package

The issue is caused by package @angular/compiler-cli/ngcc

Is this a regression?

Yes, it works in ViewEngine and should also work with Ivy after #31438 but it doesn't work for ngcc processed pipes.

Description

When using Async pipe inside a component input Angular should inject a ChangeDetectorRef of the containing view but it injects the ChangeDetectorRef that refers to component view.

@Component({
  selector: 'my-app',
  template: `
   <child [prop]="obs$ | async"></child>
  `
})
class AppComponent {}

// @angular/common
class AsyncPipe {
 constructor(cdRef: ChangeDetectorRef) {}
}

// expected  cdRef.context -> AppComponent
// actual    cdRef.context -> ChildComponent

Not too long time ago there was a PR #31438 that was intended to fix that problem with pipes. It introduced 傻傻injectPipeChangeDetectorRef function to inject ChangeDetectorRef so that the factory for pipe that injects ChangeDetectorRef should look like:

AsyncPipe.傻fac = function AsyncPipe_Factory(t) {
  return new (t || AsyncPipe)(傻ngcc0.傻傻injectPipeChangeDetectorRef(};
}

but we get the following:

AsyncPipe.傻fac = function AsyncPipe_Factory(t) { 
  return new (t || AsyncPipe)(傻ngcc0.傻傻directiveInject(ChangeDetectorRef));
};

It should be noted that ngtsc generates correct factories for user-defined pipes since in this case ngtsc uses TypeScriptReflectionHost to read metadata. The TypeScriptReflectionHost treats ChangeDetectorRef reference as ExternalExpr while Esm2015ReflectionHost treats it as a local reference

{local: true as true, expression: typeExpression, defaultImportStatement: null} :

馃實 Your Environment

Angular Version:

@angular/core@9.0.0-rc.0
@angular/common@9.0.0-rc.0
@angular/compiler-cli@9.0.0-rc.0

@alxhub @petebacondarwin @JoostK @crisbeto

@IgorMinar IgorMinar added this to the v9-candidates milestone Nov 8, 2019
@ngbot ngbot bot removed this from the v9-candidates milestone Nov 8, 2019
@ngbot ngbot bot added this to the needsTriage milestone Nov 8, 2019
@IgorMinar IgorMinar modified the milestones: needsTriage, v9-candidates Nov 8, 2019
@kara kara modified the milestones: v9-candidates, v9-blockers Nov 14, 2019
@alxhub alxhub self-assigned this Nov 16, 2019
alxhub added a commit to alxhub/angular that referenced this issue Nov 18, 2019
The ReflectionHost supports enumeration of constructor parameters, and one
piece of information it returns describes the origin of the parameter's
type. Parameter types come in two flavors: local (the type is not imported
from anywhere) or non-local (the type comes via an import).

ngcc incorrectly classified all type parameters as 'local', because in the
source files that ngcc processes the type parameter is a real ts.Identifer.
However, that identifier may still have come from an import and thus might
be non-local.

This commit changes ngcc's ReflectionHost(s) to properly recognize and
report these non-local type references.

Fixes angular#33677
alxhub added a commit to alxhub/angular that referenced this issue Nov 18, 2019
The ReflectionHost supports enumeration of constructor parameters, and one
piece of information it returns describes the origin of the parameter's
type. Parameter types come in two flavors: local (the type is not imported
from anywhere) or non-local (the type comes via an import).

ngcc incorrectly classified all type parameters as 'local', because in the
source files that ngcc processes the type parameter is a real ts.Identifer.
However, that identifier may still have come from an import and thus might
be non-local.

This commit changes ngcc's ReflectionHost(s) to properly recognize and
report these non-local type references.

Fixes angular#33677
alxhub added a commit to alxhub/angular that referenced this issue Nov 18, 2019
The ReflectionHost supports enumeration of constructor parameters, and one
piece of information it returns describes the origin of the parameter's
type. Parameter types come in two flavors: local (the type is not imported
from anywhere) or non-local (the type comes via an import).

ngcc incorrectly classified all type parameters as 'local', because in the
source files that ngcc processes the type parameter is a real ts.Identifer.
However, that identifier may still have come from an import and thus might
be non-local.

This commit changes ngcc's ReflectionHost(s) to properly recognize and
report these non-local type references.

Fixes angular#33677
gkalpak added a commit to gkalpak/angular that referenced this issue Nov 19, 2019
The ReflectionHost supports enumeration of constructor parameters, and one
piece of information it returns describes the origin of the parameter's
type. Parameter types come in two flavors: local (the type is not imported
from anywhere) or non-local (the type comes via an import).

ngcc incorrectly classified all type parameters as 'local', because in the
source files that ngcc processes the type parameter is a real ts.Identifer.
However, that identifier may still have come from an import and thus might
be non-local.

This commit changes ngcc's ReflectionHost(s) to properly recognize and
report these non-local type references.

Fixes angular#33677
@alxhub alxhub closed this in 29b8666 Nov 19, 2019
alxhub added a commit that referenced this issue Nov 19, 2019
The ReflectionHost supports enumeration of constructor parameters, and one
piece of information it returns describes the origin of the parameter's
type. Parameter types come in two flavors: local (the type is not imported
from anywhere) or non-local (the type comes via an import).

ngcc incorrectly classified all type parameters as 'local', because in the
source files that ngcc processes the type parameter is a real ts.Identifer.
However, that identifier may still have come from an import and thus might
be non-local.

This commit changes ngcc's ReflectionHost(s) to properly recognize and
report these non-local type references.

Fixes #33677

PR Close #33901
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can鈥檛 perform that action at this time.