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

feat(compiler-cli): lower metadata `useValue` and `data` literal fields #18905

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@chuckjaz
Copy link
Member

commented Aug 28, 2017

With this commit the compiler will "lower" expressions into exported
variables for values the compiler does not need to know statically
to be able to generate a factory. For example:

  providers: [{provider: 'token', useValue: calculated()}]

used to be an error as the expression calculated() is not supported
by the compiler because calculated is not a
known function

With this commit this is rewritten, during emit of the .js file, into
something like:

export var ɵ0 = calculated();

  ...

  provdiers: [{provider: 'token', useValue: ɵ0}]

the compiler then will generate a reference to the exported ɵ0 instead
of failing to evaluate calculated().

PR Type

What kind of change does this PR introduce?

[x] Feature

What is the current behavior?

An a reference to a local value, a call expression, new expression, or a lambda in a provider metadata will always cause a compile time error.

What is the new behavior?

Arbitrary expressions are allowed following a useValue, useFactory or data field in metadata as they are rewritten, during emit, to a variable exported from the module allowing the compiler to import the variable without needing to understand the expression.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@googlebot googlebot added the cla: yes label Aug 28, 2017

@chuckjaz chuckjaz force-pushed the chuckjaz:useX-lowering branch from 8da3848 to 0918eec Aug 28, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Aug 28, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Aug 29, 2017

@tbosch

tbosch approved these changes Aug 29, 2017

const ident = node as ts.Identifier;

if (!exportTable) {
exportTable = new Set();

This comment has been minimized.

Copy link
@tbosch

tbosch Aug 29, 2017

Member

maybe extract into separate function for formatting reasons.

function expectNoDiagnostics(diagnostics: Diagnostic[]) {
if (diagnostics && diagnostics.length) {
throw new Error(diagnostics.map(d => `${d.span}: ${d.message}`).join('\n'));
}
}

class TypeCheckMockAotCompilerHost extends MockAotCompilerHost {

This comment has been minimized.

Copy link
@tbosch

tbosch Aug 29, 2017

Member

Why do we need a new class here?

{ provide: T1, useValue: calculateString() },
{ provide: T2, useFactory: () => 'someValue' },
{ provide: T3, useValue: SomeEnum.OK },
{ provide: T4, useValue: {

This comment has been minimized.

Copy link
@tbosch

tbosch Aug 29, 2017

Member

Using data inside of useValue doesn't add a testcase. Could you combine it with ROUTES? (which should fail if not analyzable)?

@@ -377,6 +381,10 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor
private _visitIdentifier(
value: o.ExternalReference, typeParams: o.Type[]|null, ctx: EmitterVisitorContext): void {
const {name, moduleName} = value;
if (this.suppressLoweringReferences && name && REWRITE_IDENT.test(name)) {

This comment has been minimized.

Copy link
@tbosch

tbosch Aug 29, 2017

Member

Maybe add a general filterIdentifier callback to the TsEmitter, instead of a special case for lowering?

feat(compiler-cli): lower metadata `useValue` and `data` literal fields
With this commit the compiler will "lower" expressions into exported
variables for values the compiler does not need to know statically
in order to be able to generate a factory. For example:

```
  providers: [{provider: 'token', useValue: calculated()}]
```

produced an error as the expression `calculated()` is not supported
by the compiler because `calculated` is not a
[known function](https://angular.io/guide/metadata#annotationsdecorators)

With this commit this is rewritten, during emit of the .js file, into
something like:

```
export var ɵ0 = calculated();

  ...

  provdiers: [{provider: 'token', useValue: ɵ0}]
```

The compiler then will now generate a reference to the exported `ɵ0`
instead of failing to evaluate `calculated()`.

@chuckjaz chuckjaz force-pushed the chuckjaz:useX-lowering branch from eaf9f1c to 9ba212f Aug 29, 2017

@mary-poppins

This comment has been minimized.

Copy link

commented Aug 29, 2017

@jasonaden jasonaden closed this in c685cc2 Aug 31, 2017

jasonaden added a commit that referenced this pull request Aug 31, 2017

vicb added a commit to vicb/angular that referenced this pull request Aug 31, 2017

feat(compiler-cli): lower metadata `useValue` and `data` literal fiel…
…ds (angular#18905)

With this commit the compiler will "lower" expressions into exported
variables for values the compiler does not need to know statically
in order to be able to generate a factory. For example:

```
  providers: [{provider: 'token', useValue: calculated()}]
```

produced an error as the expression `calculated()` is not supported
by the compiler because `calculated` is not a
[known function](https://angular.io/guide/metadata#annotationsdecorators)

With this commit this is rewritten, during emit of the .js file, into
something like:

```
export var ɵ0 = calculated();

  ...

  provdiers: [{provider: 'token', useValue: ɵ0}]
```

The compiler then will now generate a reference to the exported `ɵ0`
instead of failing to evaluate `calculated()`.

PR Close angular#18905

jasonaden added a commit that referenced this pull request Aug 31, 2017

feat(compiler-cli): lower metadata `useValue` and `data` literal fiel…
…ds (#18905)

With this commit the compiler will "lower" expressions into exported
variables for values the compiler does not need to know statically
in order to be able to generate a factory. For example:

```
  providers: [{provider: 'token', useValue: calculated()}]
```

produced an error as the expression `calculated()` is not supported
by the compiler because `calculated` is not a
[known function](https://angular.io/guide/metadata#annotationsdecorators)

With this commit this is rewritten, during emit of the .js file, into
something like:

```
export var ɵ0 = calculated();

  ...

  provdiers: [{provider: 'token', useValue: ɵ0}]
```

The compiler then will now generate a reference to the exported `ɵ0`
instead of failing to evaluate `calculated()`.

PR Close #18905
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.