Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions packages/compiler-cli/src/ngtsc/annotations/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,21 +68,9 @@ export function getConstructorDependencies(
const importedSymbol = reflector.getImportOfIdentifier(tokenExpr);
if (importedSymbol !== null && importedSymbol.from === '@angular/core') {
switch (importedSymbol.name) {
case 'ChangeDetectorRef':
resolved = R3ResolvedDependencyType.ChangeDetectorRef;
break;
case 'ElementRef':
resolved = R3ResolvedDependencyType.ElementRef;
break;
case 'Injector':
resolved = R3ResolvedDependencyType.Injector;
break;
case 'TemplateRef':
resolved = R3ResolvedDependencyType.TemplateRef;
break;
case 'ViewContainerRef':
resolved = R3ResolvedDependencyType.ViewContainerRef;
break;
case 'Renderer2':
resolved = R3ResolvedDependencyType.Renderer2;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,49 @@ describe('compiler compliance', () => {
expectEmit(source, HostBindingCompDeclaration, 'Invalid host binding code');
});

it('should not treat ElementRef, ViewContainerRef, or ChangeDetectorRef specially when injecting',
() => {
const files = {
app: {
'spec.ts': `
import {Component, NgModule, ElementRef, ChangeDetectorRef, ViewContainerRef} from '@angular/core';

@Component({
selector: 'my-component',
template: ''
})
export class MyComponent {
constructor(public el: ElementRef, public vcr: ViewContainerRef, public cdr: ChangeDetectorRef) {}
}

@NgModule({declarations: [MyComponent]})
export class MyModule {}
`
}
};

const MyComponentDefinition = `
…
MyComponent.ngComponentDef = $r3$.ɵdefineComponent({
type: MyComponent,
selectors: [["my-component"]],
factory: function MyComponent_Factory(t) {
return new (t || MyComponent)(
$r3$.ɵdirectiveInject(ElementRef), $r3$.ɵdirectiveInject(ViewContainerRef),
$r3$.ɵdirectiveInject(ChangeDetectorRef));
},
features: [$r3$.ɵPublicFeature],
consts: 0,
vars: 0,
template: function MyComponent_Template(rf, ctx) {}
});`;

const result = compile(files, angularFiles);
const source = result.source;

expectEmit(source, MyComponentDefinition, 'Incorrect MyComponent.ngComponentDef');
});

it('should support structural directives', () => {
const files = {
app: {
Expand Down Expand Up @@ -677,7 +720,7 @@ describe('compiler compliance', () => {
IfDirective.ngDirectiveDef = $r3$.ɵdefineDirective({
type: IfDirective,
selectors: [["", "if", ""]],
factory: function IfDirective_Factory(t) { return new (t || IfDirective)($r3$.ɵinjectTemplateRef()); },
factory: function IfDirective_Factory(t) { return new (t || IfDirective)($r3$.ɵdirectiveInject(TemplateRef)); },
features: [$r3$.ɵPublicFeature]
});`;
const MyComponentDefinition = `
Expand Down Expand Up @@ -1703,7 +1746,7 @@ describe('compiler compliance', () => {
type: ForOfDirective,
selectors: [["", "forOf", ""]],
factory: function ForOfDirective_Factory(t) {
return new (t || ForOfDirective)($r3$.ɵinjectViewContainerRef(), $r3$.ɵinjectTemplateRef());
return new (t || ForOfDirective)($r3$.ɵdirectiveInject(ViewContainerRef), $r3$.ɵdirectiveInject(TemplateRef));
},
features: [$r3$.ɵPublicFeature, $r3$.ɵNgOnChangesFeature],
inputs: {forOf: "forOf"}
Expand Down Expand Up @@ -1779,7 +1822,7 @@ describe('compiler compliance', () => {
type: ForOfDirective,
selectors: [["", "forOf", ""]],
factory: function ForOfDirective_Factory(t) {
return new (t || ForOfDirective)($r3$.ɵinjectViewContainerRef(), $r3$.ɵinjectTemplateRef());
return new (t || ForOfDirective)($r3$.ɵdirectiveInject(ViewContainerRef), $r3$.ɵdirectiveInject(TemplateRef));
},
features: [$r3$.ɵPublicFeature, $r3$.ɵNgOnChangesFeature],
inputs: {forOf: "forOf"}
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ describe('ngtsc behavioral tests', () => {
const jsContents = getContents('test.js');
expect(jsContents)
.toContain(
`factory: function FooCmp_Factory(t) { return new (t || FooCmp)(i0.ɵinjectAttribute("test"), i0.ɵinjectChangeDetectorRef(), i0.ɵinjectElementRef(), i0.ɵdirectiveInject(i0.INJECTOR), i0.ɵinjectRenderer2(), i0.ɵinjectTemplateRef(), i0.ɵinjectViewContainerRef()); }`);
`factory: function FooCmp_Factory(t) { return new (t || FooCmp)(i0.ɵinjectAttribute("test"), i0.ɵdirectiveInject(ChangeDetectorRef), i0.ɵdirectiveInject(ElementRef), i0.ɵdirectiveInject(i0.INJECTOR), i0.ɵinjectRenderer2(), i0.ɵdirectiveInject(TemplateRef), i0.ɵdirectiveInject(ViewContainerRef)); }`);
});

it('should generate queries for components', () => {
Expand Down
40 changes: 3 additions & 37 deletions packages/compiler/src/render3/r3_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,30 +101,10 @@ export enum R3ResolvedDependencyType {
*/
Injector = 2,

/**
* The dependency is for `ElementRef`.
*/
ElementRef = 3,

/**
* The dependency is for `TemplateRef`.
*/
TemplateRef = 4,

/**
* The dependency is for `ViewContainerRef`.
*/
ViewContainerRef = 5,

/**
* The dependency is for `ChangeDetectorRef`.
*/
ChangeDetectorRef = 6,

/**
* The dependency is for `Renderer2`.
*/
Renderer2 = 7,
Renderer2 = 3,
}

/**
Expand Down Expand Up @@ -282,14 +262,6 @@ function compileInjectDependency(
case R3ResolvedDependencyType.Attribute:
// In the case of attributes, the attribute name in question is given as the token.
return o.importExpr(R3.injectAttribute).callFn([dep.token]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you thought about how we could get rid of injectAttribute? I think calling @Attribute('name') should create a token which captures the name and than writes a factory into NG_ELEMENT_ID where the name is captured.

Given:

class MyDirective {
  constructor(@Atribute('name') name) {}
}

should generate:

factory: () => new MyDirective(directiveInject(Attribute('name'));

so that Attribute

function Attribute(name: string) {
  return {
    attributeName: name,
    __NG_ELEMENT_ID__: () => injectAttribute(name)
  }
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This sounds good, but let's push to another PR?

case R3ResolvedDependencyType.ElementRef:
return o.importExpr(R3.injectElementRef).callFn([]);
case R3ResolvedDependencyType.TemplateRef:
return o.importExpr(R3.injectTemplateRef).callFn([]);
case R3ResolvedDependencyType.ViewContainerRef:
return o.importExpr(R3.injectViewContainerRef).callFn([]);
case R3ResolvedDependencyType.ChangeDetectorRef:
return o.importExpr(R3.injectChangeDetectorRef).callFn([]);
case R3ResolvedDependencyType.Renderer2:
return o.importExpr(R3.injectRenderer2).callFn([]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you thought about how we could get rid of injectRenderer2? I think adding the NG_ELEMENT_ID should do the trick.

default:
Expand Down Expand Up @@ -320,13 +292,7 @@ export function dependenciesFromGlobalMetadata(
if (dependency.token) {
const tokenRef = tokenReference(dependency.token);
let resolved: R3ResolvedDependencyType = R3ResolvedDependencyType.Token;
if (tokenRef === elementRef) {
resolved = R3ResolvedDependencyType.ElementRef;
} else if (tokenRef === templateRef) {
resolved = R3ResolvedDependencyType.TemplateRef;
} else if (tokenRef === viewContainerRef) {
resolved = R3ResolvedDependencyType.ViewContainerRef;
} else if (tokenRef === injectorRef) {
if (tokenRef === injectorRef) {
resolved = R3ResolvedDependencyType.Injector;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you thought about how we could get rid of injectInjector? I think adding NG_ELEMENT_ID should do the trick.

} else if (tokenRef === renderer2) {
resolved = R3ResolvedDependencyType.Renderer2;
Expand Down Expand Up @@ -363,4 +329,4 @@ function isDelegatedMetadata(meta: R3FactoryMetadata): meta is R3DelegatedFactor

function isExpressionFactoryMetadata(meta: R3FactoryMetadata): meta is R3ExpressionFactoryMetadata {
return (meta as any).expression !== undefined;
}
}
10 changes: 0 additions & 10 deletions packages/compiler/src/render3/r3_identifiers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,6 @@ export class Identifiers {

static injectAttribute: o.ExternalReference = {name: 'ɵinjectAttribute', moduleName: CORE};

static injectElementRef: o.ExternalReference = {name: 'ɵinjectElementRef', moduleName: CORE};

static injectTemplateRef: o.ExternalReference = {name: 'ɵinjectTemplateRef', moduleName: CORE};

static injectViewContainerRef:
o.ExternalReference = {name: 'ɵinjectViewContainerRef', moduleName: CORE};

static injectChangeDetectorRef:
o.ExternalReference = {name: 'ɵinjectChangeDetectorRef', moduleName: CORE};

static injectRenderer2: o.ExternalReference = {name: 'ɵinjectRenderer2', moduleName: CORE};

static directiveInject: o.ExternalReference = {name: 'ɵdirectiveInject', moduleName: CORE};
Expand Down
2 changes: 1 addition & 1 deletion packages/compiler/src/render3/view/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function baseDirectiveFields(
definitionMap.set('selectors', createDirectiveSelector(meta.selector !));


// e.g. `factory: () => new MyApp(injectElementRef())`
// e.g. `factory: () => new MyApp(directiveInject(ElementRef))`
const result = compileFactoryFunction({
name: meta.name,
type: meta.type,
Expand Down
34 changes: 23 additions & 11 deletions packages/core/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@ ng_module(
"*.ts",
"src/**/*.ts",
],
exclude = ["src/ivy_switch.ts"],
exclude = [
"src/ivy_switch/compiler/index.ts",
"src/ivy_switch/runtime/index.ts",
],
) + [
":ivy_switch",
":ivy_switch_compiler",
":ivy_switch_runtime",
],
module_name = "@angular/core",
deps = [
Expand Down Expand Up @@ -42,19 +46,27 @@ ng_package(

## Controls if Ivy is enabled. (Temporary target until we permanently switch over to Ivy)
##
## This file generates `src/ivy_switch.ts` file which reexports symbols for `ViewEngine` or `Ivy.`
## - append `--define=compile=legacy` (default) to `bazel` command to reexport `./ivy_switch_legacy`
## and use `ViewEngine`
## - append `--define=compile=jit` to `bazel` command to rexport `./ivy_switch_jit` and use `Ivy`
## - append `--define=compile=local` to `bazel` command to rexport `./ivy_switch_jit` and use `Ivy`
## This file generates the `src/ivy_switch/compiler/index.ts` and `src/ivy_switch/runtime/index.ts` files which
## reexport symbols for `ViewEngine` or `Ivy.`
## - append `--define=compile=legacy` (default) to `bazel` command to reexport `./legacy` from each folder
## in the 'ivy_switch' directory and use `ViewEngine`
## - append `--define=compile=jit` to `bazel` command to rexport `./jit` from each folder in the `ivy_switch`
## directory and use `Ivy`
## - append `--define=compile=local` to `bazel` command to rexport `./ivy_switch/compiler/jit` and use `Ivy`
## in the local analysis mode. (run as part of `ngtsc`)
##
## NOTE: `--define=compile=jit` works with any `bazel` command or target across the repo.
##
## See: `//tools/bazel.rc` where `--define=ivy=false` is defined as default.
## See: `./src/ivy_switch.ts` for more details.
## See: `./src/ivy_switch/compiler/index.ts` for more details.
genrule(
name = "ivy_switch_compiler",
outs = ["src/ivy_switch/compiler/index.ts"],
cmd = "echo export '*' from \"'./$(compile)';\" > $@",
)

genrule(
name = "ivy_switch",
outs = ["src/ivy_switch.ts"],
cmd = "echo export '*' from \"'./ivy_switch_$(compile)';\" > $@",
name = "ivy_switch_runtime",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the reasoning for this change?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The compiler switch relies on jit/directive <-- jit/environment <-- render3/index. So any file that has imports/exports in render3/index.ts (most render3 files) is importing into ivy_switch. Separating it out into its own file gives us a bit more flexibility.

outs = ["src/ivy_switch/runtime/index.ts"],
cmd = "echo export '*' from \"'./$(compile)';\" > $@",
)
5 changes: 5 additions & 0 deletions packages/core/src/change_detection/change_detector_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/

import {R3_CHANGE_DETECTOR_REF_FACTORY} from '../ivy_switch/runtime/index';

/**
* Base class for Angular Views, provides change detection functionality.
* A change-detection tree collects all views that are to be checked for changes.
Expand Down Expand Up @@ -103,4 +105,7 @@ export abstract class ChangeDetectorRef {
*
*/
abstract reattach(): void;

/** @internal */
static __NG_ELEMENT_ID__: () => ChangeDetectorRef = () => R3_CHANGE_DETECTOR_REF_FACTORY();
}
2 changes: 1 addition & 1 deletion packages/core/src/core_private_export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export {Console as ɵConsole} from './console';
export {InjectableDef as ɵInjectableDef, InjectorDef as ɵInjectorDef, getInjectableDef as ɵgetInjectableDef} from './di/defs';
export {inject as ɵinject, setCurrentInjector as ɵsetCurrentInjector} from './di/injector';
export {APP_ROOT as ɵAPP_ROOT} from './di/scope';
export {ivyEnabled as ɵivyEnabled} from './ivy_switch';
export {ivyEnabled as ɵivyEnabled} from './ivy_switch/compiler/index';
export {ComponentFactory as ɵComponentFactory} from './linker/component_factory';
export {CodegenComponentFactoryResolver as ɵCodegenComponentFactoryResolver} from './linker/component_factory_resolver';
export {resolveComponentResources as ɵresolveComponentResources} from './metadata/resource_loading';
Expand Down
17 changes: 11 additions & 6 deletions packages/core/src/core_render3_private_export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ export {
DirectiveType as ɵDirectiveType,
RenderFlags as ɵRenderFlags,
directiveInject as ɵdirectiveInject,
injectElementRef as ɵinjectElementRef,
injectTemplateRef as ɵinjectTemplateRef,
injectViewContainerRef as ɵinjectViewContainerRef,
injectChangeDetectorRef as ɵinjectChangeDetectorRef,
injectRenderer2 as ɵinjectRenderer2,
injectAttribute as ɵinjectAttribute,
getFactoryOf as ɵgetFactoryOf,
Expand Down Expand Up @@ -187,6 +183,15 @@ export {
R3_COMPILE_INJECTABLE__POST_NGCC__ as ɵR3_COMPILE_INJECTABLE__POST_NGCC__,
R3_COMPILE_NGMODULE__POST_NGCC__ as ɵR3_COMPILE_NGMODULE__POST_NGCC__,
R3_COMPILE_PIPE__POST_NGCC__ as ɵR3_COMPILE_PIPE__POST_NGCC__,
ivyEnable__POST_NGCC__ as ɵivyEnable__POST_NGCC__
} from './ivy_switch_legacy';
ivyEnable__POST_NGCC__ as ɵivyEnable__POST_NGCC__,
} from './ivy_switch/compiler/legacy';

export {
R3_ELEMENT_REF_FACTORY__POST_NGCC__ as ɵR3_ELEMENT_REF_FACTORY__POST_NGCC__,
R3_TEMPLATE_REF_FACTORY__POST_NGCC__ as ɵR3_TEMPLATE_REF_FACTORY__POST_NGCC__,
R3_CHANGE_DETECTOR_REF_FACTORY__POST_NGCC__ as ɵR3_CHANGE_DETECTOR_REF_FACTORY__POST_NGCC__,
R3_VIEW_CONTAINER_REF_FACTORY__POST_NGCC__ as ɵR3_VIEW_CONTAINER_REF_FACTORY__POST_NGCC__,
} from './ivy_switch/runtime/legacy';


// clang-format on
2 changes: 1 addition & 1 deletion packages/core/src/di/injectable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {R3_COMPILE_INJECTABLE} from '../ivy_switch';
import {R3_COMPILE_INJECTABLE} from '../ivy_switch/compiler/index';
import {Type} from '../type';
import {makeDecorator} from '../util/decorators';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
* For more information on how to run and debug tests with either Ivy or View Engine (legacy),
* please see [BAZEL.md](./docs/BAZEL.md).
*/
export * from './ivy_switch_legacy';
export * from './legacy';

// TODO(alxhub): debug why metadata doesn't properly propagate through this file.
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@
* found in the LICENSE file at https://angular.io/license
*/

import {compileComponent, compileDirective} from './render3/jit/directive';
import {compileInjectable} from './render3/jit/injectable';
import {compileNgModule} from './render3/jit/module';
import {compilePipe} from './render3/jit/pipe';
import {compileComponent, compileDirective} from '../../render3/jit/directive';
import {compileInjectable} from '../../render3/jit/injectable';
import {compileNgModule} from '../../render3/jit/module';
import {compilePipe} from '../../render3/jit/pipe';

export const ivyEnabled = true;
export const R3_COMPILE_COMPONENT = compileComponent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@
* found in the LICENSE file at https://angular.io/license
*/

import {InjectableType, InjectorType, defineInjectable, defineInjector, getInjectableDef} from './di/defs';
import {InjectableProvider} from './di/injectable';
import {inject, injectArgs} from './di/injector';
import {ClassSansProvider, ConstructorSansProvider, ExistingSansProvider, FactorySansProvider, StaticClassSansProvider, ValueProvider, ValueSansProvider} from './di/provider';
import {InjectableType, InjectorType, defineInjectable, defineInjector, getInjectableDef} from '../../di/defs';
import {InjectableProvider} from '../../di/injectable';
import {inject, injectArgs} from '../../di/injector';
import {ClassSansProvider, ConstructorSansProvider, ExistingSansProvider, FactorySansProvider, StaticClassSansProvider, ValueProvider, ValueSansProvider} from '../../di/provider';
import {NgModule} from '../../metadata';
import {ReflectionCapabilities} from '../../reflection/reflection_capabilities';
import {Type} from '../../type';
import {getClosureSafeProperty} from '../../util/property';

import * as ivyOn from './ivy_switch_on';
import {NgModule} from './metadata';
import {ReflectionCapabilities} from './reflection/reflection_capabilities';
import {Type} from './type';
import {getClosureSafeProperty} from './util/property';

function noop() {}

Expand All @@ -25,13 +26,15 @@ export const R3_COMPILE_DIRECTIVE__POST_NGCC__: DirectiveCompiler = ivyOn.R3_COM
export const R3_COMPILE_INJECTABLE__POST_NGCC__: DirectiveCompiler = ivyOn.R3_COMPILE_INJECTABLE;
export const R3_COMPILE_NGMODULE__POST_NGCC__: DirectiveCompiler = ivyOn.R3_COMPILE_NGMODULE;
export const R3_COMPILE_PIPE__POST_NGCC__: DirectiveCompiler = ivyOn.R3_COMPILE_PIPE;

export const ivyEnable__POST_NGCC__: boolean = ivyOn.ivyEnabled;

const R3_COMPILE_COMPONENT__PRE_NGCC__: DirectiveCompiler = noop;
const R3_COMPILE_DIRECTIVE__PRE_NGCC__: DirectiveCompiler = noop;
const R3_COMPILE_INJECTABLE__PRE_NGCC__: DirectiveCompiler = preR3InjectableCompile;
const R3_COMPILE_NGMODULE__PRE_NGCC__: DirectiveCompiler = preR3NgModuleCompile;
const R3_COMPILE_PIPE__PRE_NGCC__: DirectiveCompiler = noop;

const ivyEnable__PRE_NGCC__ = false;

export const ivyEnabled = ivyEnable__PRE_NGCC__;
Expand Down Expand Up @@ -111,4 +114,4 @@ function preR3InjectableCompile(
factory: convertInjectableProviderToFactory(injectableType, options),
});
}
}
}
Loading