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

Codegen App Injector / remove ReflectiveInjector and Reflector for offline compile #8997

Closed
tbosch opened this issue Jun 3, 2016 · 25 comments

Comments

@tbosch
Copy link
Contributor

tbosch commented Jun 3, 2016

Angular is already generating Injectors on elements for components. We would like to have the same approach for the injector used for bootstrapping and the injector used for dynamic component loading.

With this, we no longer need any metadata about classes / functions in offline compile mode, as all of the metadata (decorators) have been read during offline compile and converted into appropriate code. This means:

  • Download size impact:
    • We are no longer using ReflectiveInjector nor Reflector / ReflectionCapabilities
      • this drops about 30kb minified / 6.49k minified gzipped from angular core
    • we don't need the reflect-metadata polyfill anymore when during offline compile (2.1kb min gzipped)
    • we don't need to keep the decorators anymore during runtime.
    • we don't need to store the constructor arguments any more
    • we are generting code for the injectors instead, which needs to be measured how much.
  • Bootstrap speed
    • analyzing the dependencies of classes, normalizing them and creating factories for them during bootstrap of an application can be costly depending on the number of services used. This especially affects bootstrap as the JavaScript vm most probably did not have time to optimize the code.
    • this was proven by internal benchmarks for the last bigger change to DI for elements (see #8b000a7f0e9fade5e6a9354532597018c5ad9a55).
  • Better global static analyzability of injection of angular apps. In the future, this would allow to check which providers are actually used in an application and report missing providers that services / directives that are used depend upon.

Impact for users:

  • This change does not affect runtime compile, only offline compilation.
  • This change only affects how users add additional providers to bootstrap / when components are dynamically loaded. Bootrapping / dynamically loading components without additional providers does not change.
  • This change does not change how components use providers

Part of #6270

Input and output of the compilation

Declare injectors via a config class like this:

@InjectorConfig({
  providers: [SomeService]
})
class MyAppConfig {
  constructor(public @Provides(SomeToken) someProp: string) {
  }
}

Via runtime or offline compile, we create a InjectorFactory that is usable like this:

var injectorFactory: InjectorFactory<MyAppConfig>;
var parentInjector: Injector;

var injector: Injector = injectorFactory.create(parentInjector, new MyAppConfig('hello'));
expect(injector.get(SomeService)).toBeAndInstanceOf(SomeService);
expect(injector.get(SomeToken)).toBe('hello');

Usage with offline compilation

// Name: drop suffix `Config` if existing and add `InjectorFactory`
import {MyAppInjectorFactory} from 'my_config.ngfactory';

MyAppInjectorFactory.create(...);

Usage with runtime compilation

var componentResolver: ComponentResolver;

// This would also jit the generated code, similar to our component compiler.
componentResolver.resolveInjector(MyAppConfig).then( 
    (injectorFactory: InjectorFactory<MyAppConfig>) => {
      injectorFactory.create(...);
    }
);

Consequences

  • We can still support the bootstrap(type, providers) call in @angular/browser as this uses the RuntimeCompiler already, so it can also create a injector config class on the fly with the given providers. -> plunkers stay simpler.
  • To create the ComponentResolver, we can't use DI any more, as it provides the factory for injectors.
  • As the compiler creates the injector factories, the compiler can't be configured via DI, but we rather need factory methods that take the CompilerConfig and maybe some other parameters.
    • e.g. platform directives / platform pipes need to become part of the CompilerConfig,
      and not provided via DI (this also unifies their configuration between offline and runtime compile).

Proposed changes to Angular besides the compiler changes

  • [BREAKING] move ReflectiveInjector and move it to @angular/common or into a
    separate module as Angular itself does no more use it.
    • It is still used by e.g. benchpress so we need to keep it.
    • Instead, users have to create injector config classes as shown above for both, runtime and offline compile. For runtime compile, we could create these injector config classes on the fly, but to make it easier for users to switch to offline compile they should already use them.
    • or maybe just deprecate it?
  • move Reflector and ReflectionCapabilities from @angular/core to @angular/compiler as the
    compiler is the only place where these are used.
  • make ComponentResolver (an RuntimeCompiler) part of PlatformRef and don't use DI to create the providers of PlatformRef. Still expose an injector in PlatformRef which is just based on a Map.
  • [BREAKING] make platform pipes / platform directives part of CompilerConfig and define them as arrays of CompileIdentifierMetadata so they can be used for offline and runtime compile.
  • [BREAKING] pass CompilerConfig to browserPlatform() as argument instead of getting it via DI.
    • we might consider using a special BrowserCompileConfig class as some parts of CompilerConfig are implied by using the browser platform (e.g. that platform directives are always defined via their runtime types, i.e. the user only needs to provide an array of types instead of CompileIdentifierMetadata)
@tbosch
Copy link
Contributor Author

tbosch commented Jun 3, 2016

/cc @mhevery

@tbosch
Copy link
Contributor Author

tbosch commented Jun 3, 2016

Other changes agreed upon:

  • Rename ComponentResolver to FactoryResolver
  • Change the exported name of generated ComponentFactorys to not include Ng in the name, e.g. MyComponent -> MyComponentFactory, but keep the filename as is (i.e. ...ngfactory.ts).
  • Move ReflectiveInjector into benchpress for now until we decide what to do with it.

@mhevery
Copy link
Contributor

mhevery commented Jun 3, 2016

Reviewed with @tbosch and agreeing with the approach.

@wardbell
Copy link
Contributor

wardbell commented Jun 3, 2016

Please provide an illustration of how this affects the way I bootstrap my app and how I provide to components (especially AppComponent).

Imagine this (partial) application (semi-pseudo-coded):

// main.ts
bootstrap(AppComponent, [HTTP_PROVIDERS]);

// hero.service.ts
export class HeroService {
  constructor(private http: Http) { }
  // ... the rest of it
}

// hero-list.component.ts
@Component({
  selector: 'hero-list',
  template: '<div *ngFor="let hero of heroes>{{hero.name}}</div>'
})
export class HeroListComponent() { 
  constructor(private heroService: HeroService) { }
  // ... the rest of it
}

// app.component.ts
@Component({
  selector: 'my-app',
  template: '<hero-list></hero-list>',
  providers: [HeroService]
})
export class AppComponent() { }

@mhevery
Copy link
Contributor

mhevery commented Jun 3, 2016

@wardbell bootstrap forces in browser compilation, so the above would not change. This will only come into play with offline-compilation.

tbosch added a commit to tbosch/angular that referenced this issue Jun 3, 2016
@wardbell
Copy link
Contributor

wardbell commented Jun 4, 2016

That's what we all wanted to hear :-)

@tbosch
Copy link
Contributor Author

tbosch commented Jun 6, 2016

The pattern of a config that combines a set of statically known providers with a set of unknown providers is also usable for defining the configuration of a library.

E.g.

@InjectorConfig({
  providers: [locationProvider, ...]
})
class RouterConfig {
  @Provides(RouterBasePath)
  basePath: string;
}

This corresponds to the pattern of creating provider functions like this

function routerProviders(basePath: string):any[] {
  return [
    locationProvider, ...,
    provide(RouterBasePath, useValue: basePath)
  ];
}

However, in this case, the config is only part of the total config of an application. The encapsulation into a Config class is used because:

  • the user only needs to know about 1 thing to configure DI for a library, even if the library needs some input tokens that the user should provide
  • the typechecker can check that all input tokens are provided and are of the right type

These benefits show more the more input tokens a library has

The following proposes a way to compose configurations:

@InjectorConfig()
class AppConfig {
  @Configures(RouterConfig)
  routerConfig(dep: SomeDependency): RouterConfig {
    return new RouterConfig('/...');
  }
}

I.e. we support method injection to create Config objects and automatically include the static providers of these configs in the main injector as well.

In components, configs can also be used as follows:

@Component({
  providers: [{
    configure: RouterConfig,
    useValue: new RouterConfig('/...')
  }]
})
class SomeComp {}

Configs can be included in other configs or providers of components / directives via @Configures (to include into configs) or {configures: ...} for components / directives. Syntax:
@Configures(<ConfigClass>, {useValue: ..., useFactory: ...});

@tbosch
Copy link
Contributor Author

tbosch commented Jun 6, 2016

Note from Igor: Right now we only have @Inject as a verb for a decorator. However, we can't use @Provider or @Injector as Angular already exports the classes Injector and Provider which would collide.

@tbosch
Copy link
Contributor Author

tbosch commented Jun 6, 2016

Maybe we can determine the argument to @Configures and @Provides using the result of the method by default, so the user has less to write.

  • TS does store the return type as __metadata('design:returntype', ...)
  • however, we don't rely on reading the return type anywhere else

@wardbell
Copy link
Contributor

wardbell commented Jun 6, 2016

This is jibberish to me. I have no idea what problem we're solving or what the consequences are.

I'm getting feedback from community confirming that this mystifies ... and frightens ... that community.

This proposal adds new terms and complexities that we were previously unaware of.

Where do we go to find out whether this is important and what it means?

@shlomiassaf
Copy link
Contributor

The proposal provides better encapsulation and better static analysis for libraries/modules, the tradeoff is adding more depth into libraries making it harder to understand them which has an impact since their usually OS projects.

The result might be simple library integration for the end user.

I admit it looks complex and a lot of new terms for DI.

tbosch added a commit to tbosch/angular that referenced this issue Jun 6, 2016
@tbosch tbosch changed the title Codegen App Injector / remove ReflectiveInjector and Reflector Codegen App Injector / remove ReflectiveInjector and Reflector for offline compile Jun 6, 2016
tbosch added a commit to tbosch/angular that referenced this issue Jun 6, 2016
This is just adding them, but not using them yet
for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 7, 2016
This is just adding them, but not using them yet
for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 7, 2016
This is just adding them, but not using them yet
for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 7, 2016
This is just adding them, but not using them yet
for bootstrap.

Related to angular#8997
@mhevery
Copy link
Contributor

mhevery commented Jun 7, 2016

@tbosch can you rewrite @wardbell 's example here #8997 (comment) with offline compilation. I think the only thing which needs to be change is the bootstrap

@tbosch
Copy link
Contributor Author

tbosch commented Jun 7, 2016

@wardbell's example would look like this with the current offline compilation:

// main.ts
import {AppComponentNgFactory} from 'app.component.ngfactory';

var appInjector = ReflectiveInjector.createAndResolve(browserPlatform().injector, [
  BROWSER_APP_PROVIDERS, HTTP_PROVIDERS
]);
coreBootstrap(AppComponentNgFactory, appInjector);

After this issue, offline compilation would look like this:

// main.ts
import {AppComponentNgFactory} from 'app.component.ngfactory';
import {AppConfigInjectorFactory} from 'app.config.ngfactory';

// app.config.ts
@InjectorConfig({
  providers: [BROWSER_APP_PROVIDERS, HTTP_PROVIDERS]
})
export class AppConfig {}

coreBootstrap(
  AppComponentNgFactory, 
  AppConfigInjectorFactory.create(browserPlatform().injector)
);


// all other files stay as is
...

I.e. calls to ReflectiveInjector.create... are replaced by defining a @InjectorConfig class + calling the InjectorFactory that is generated for that @InjectorConfig class.

@mhevery
Copy link
Contributor

mhevery commented Jun 7, 2016

@tbosch I think we should change @InjectorConfig to @Injector as the word Config is not needed.

@wardbell
Copy link
Contributor

wardbell commented Jun 7, 2016

@mhevery The likely obstacle there is that there is already an Injector type. Won't that be confused with the decorator if we need to import both?

@wardbell
Copy link
Contributor

wardbell commented Jun 7, 2016

Dear Reader

I you are like me, this issue was both confusing and frightening. What's it about? In the space of a month we went from Injector to ReflectiveInjector and now that's disappearing ... along with some other types we probably never knew existed.

@mhevery was kind enough to help me better understand (not fully understand) what this is about and the consequences for our code. My goal in this note is to corral the issue and reduce our (my?) degree of concern.

_This is my interpretation of our conversation. I'll update this note if I learn that I've got it wrong._

This change matters only for offline compilation

Our application does not have to change if it will always compile at runtime. See the app sample above and @mhevery's assurance on that score.

Of course most of us want our apps to be compilable offline. We'd feel foolish if we deliberately wrote apps that couldn't use the offline compiler. So this point isn't all that comforting.

Fortunately ...

No change to providers for our components

I'm not ready to describe the change yet. I'll get to that. What we need to know is that we keep writing components the way we've been taught. The HeroListComponent and all of the other child components still look like this:

// hero-list.component.ts
@Component({
  selector: 'hero-list',
  template: '<div *ngFor="let hero of heroes>{{hero.name}}</div>',
  providers: [HeroService]
})
export class HeroListComponent() { 
  constructor(private heroService: HeroService) { }
  // ... the rest of it
}

Notice that I'm specifying a provider here in the component metadata ... exactly as I did before! I'm not using that @InjectConfig thing described in this issue.

The @InjectConfig change affects bootstrapping only

This isn't true for everyone. But it will be true for most applications, especially in their early stages.

This means we can keep bootstrapping as we have learned to do during early development.

We need only wake up to this change when we want to enable offline template compilation.
And when we do, it means changing in our code _in one place_ ... in the place where we bootstrap the application. We'll change our main.ts ... and add a new file that we could call app.config.ts ... and we'll be done.

Here I'm confining our attention to the @InjectConfig question. I'm assuming that the rest of the application is supportive of offline compilation.

We don't have to learn this stuff or teach this stuff ... until we're ready to learn/teach offline compilation.

We won't paint ourselves into a corner. We can get to it when we get to it.

What's going on?

You know that what we call a component consists of a controller class and the metadata in @Component.

We probably guessed that the runtime template compiler doesn't (re)interpret the metadata every time it creates a new instance of HeroListComponent. The compiler creates a factory for producing instances of something (the true component object) that is the marriage of the class and the metadata. Then Angular calls this factory each time it makes a new HeroListComponent.

The runtime compiler creates this factory in memory. The offline compiler creates this factory as its own TS file. The name of that file is the name of your component file + .ngfactory (or something like that).

So when @tbosch wrote

// main.ts
import {AppComponentNgFactory} from 'app.component.ngfactory';

he was explicitly importing the factory rather than the original AppComponent class with its attached @Component metadata.

The offline compiler creates a xxx.component.ngfactory.ts for every xxx.component.ts. We'll see that in the output directory. We won't care about this in most cases. We do care about it for the root component during bootstrapping.

That makes sense to me. Normally, I just want to refer to the thing I wrote (app.component.ts). But I guess I can get used to the extra twist of importing the factory (app.component.ngfactory) ... during bootstrapping only ... as long as I don't have to think about that when I'm learning Angular.

Maybe we can sugar this away some day.

What's new here is that we must specify the root injector separately during bootstrapping.

Our app only has one root injector. Apparently, Angular can't just manufacture it for us. We have to tell Angular make it. That's what we're doing in

// app.config.ts
@InjectorConfig({
  providers: [BROWSER_APP_PROVIDERS, HTTP_PROVIDERS]
})
export class AppConfig {}

We're extracting the stuff that we used to put in the bootstrap providers array and putting that in the @InjectorConfig metadata for an application configuration class. And we're also specifying the Angular 2 browser providers our app we'll need. I'm sure we can do other goofy stuff in here as well.

Now the template compiler, as it did with our component, turns the config+inject-metadata into an injector factory. If we called our class file app.config.ts, the compiler-generated factory file will be app.config.ngfactory.

And we'll import that during bootstrapping

// main.ts
import {AppConfigInjectorFactory} from 'app.config.ngfactory';

Finally, in order to bootstrap the app, we give the offline boostrapping method both the root component factory and the root injector factory.

coreBootstrap(
  AppComponentNgFactory, 
  AppConfigInjectorFactory.create(browserPlatform().injector)
);

This starts to make sense ... even seems necessary ... when we remember that there is no runtime compiler in the browser. We can't rely on a compiler to create these factories in memory on the fly.

That's why we become aware of the factories when we use the offline compiler ... and can completely ignore them when we were using the runtime compiler.

Which is also to say that we can completely ignore them while we are learning Angular 2.

Is this useful outside of bootstrapping?

Yes. But I'll save that for another comment at another time.

@ericmartinezr
Copy link
Contributor

@wardbell Thank you so much for that, you answered exactly all the questions I had about this change. Thanks, seriously. (I know the +1 exists, but I can't express with it how much I appreciate your comment).

PS: sorry for the noise.

@wardbell
Copy link
Contributor

wardbell commented Jun 7, 2016

@ericmartinezr I wrote it for you ... and everyone like you ... which is to say ... everyone like me ;-) Glad you appreciated it.

@tapas4java
Copy link

Since it is related to offline compilation context only, Can we call it @OfflineInjector instead of @InjectorConfig or @Injector ??

@wardbell
Copy link
Contributor

wardbell commented Jun 7, 2016

@tapas4java It actually has other use cases that I have not covered and that are not limited to offline compiler. So I don't think that's a healthy move.

tbosch added a commit to tbosch/angular that referenced this issue Jun 7, 2016
This commit is just adding them, but not using them yet for bootstrap.

Related to angular#8997
@tbosch
Copy link
Contributor Author

tbosch commented Jun 7, 2016

We talked about this internally, and concluded that we don't do the part that allows to reuse InjectorConfigs in other configs or providers for now.

tbosch added a commit to tbosch/angular that referenced this issue Jun 7, 2016
This commit is just adding them, but not using them yet for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 7, 2016
This commit is just adding them, but not using them yet for bootstrap.

Related to angular#8997
@wardbell
Copy link
Contributor

wardbell commented Jun 7, 2016

@tbosch To clarify, the plan had been that we would be able to use the same @InjectorConfig mechanism to

  1. establish separate injector trees for router configuration guards
  2. separate injector trees for 3rd party packages (e.g., a component library) that need to inject their own services prior to usage.

Are these use cases off the table? For now? Forever?

@mhevery
Copy link
Contributor

mhevery commented Jun 8, 2016

@wardbell We needed this to code gen the injectors. The benefit would be faster startup and smaller payload. For now we decided to keep the reflective injectors. We may revisit this at some later point in time.

tbosch added a commit to tbosch/angular that referenced this issue Jun 9, 2016
This commit is just adding them, but not using them yet for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 9, 2016
This commit is just adding them, but not using them yet for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 13, 2016
This commit is just adding them, but not using them yet for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 13, 2016
This commit is just adding them, but not using them yet for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 14, 2016
This commit is just adding them, but not using them yet for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 14, 2016
This commit is just adding them, but not using them yet for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 14, 2016
This commit is just adding them, but not using them yet for bootstrap.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 14, 2016
Introduce @InjectorModule and generate `InjectorFactory`s out of them.

Support using `@InjectorModule`s in any place where providers are supported.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 14, 2016
Introduce @InjectorModule and generate `InjectorFactory`s out of them.

Support using `@InjectorModule`s in any place where providers are supported.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 14, 2016
Introduce @InjectorModule and generate `InjectorFactory`s out of them.

Support using `@InjectorModule`s in any place where providers are supported.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 14, 2016
Introduce @InjectorModule and generate `InjectorFactory`s out of them.

Support using `@InjectorModule`s in any place where providers are supported.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 14, 2016
Introduce @InjectorModule and generate `InjectorFactory`s out of them.

Support using `@InjectorModule`s in any place where providers are supported.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 14, 2016
Introduce @InjectorModule and generate `InjectorFactory`s out of them.

Support using `@InjectorModule`s in any place where providers are supported.

Related to angular#8997
tbosch added a commit to tbosch/angular that referenced this issue Jun 14, 2016
Introduce @InjectorModule and generate `InjectorFactory`s out of them.

Support using `@InjectorModule`s in any place where providers are supported.

Related to angular#8997
@tbosch
Copy link
Contributor Author

tbosch commented Jul 7, 2016

We have AppModules now, which generate the bootstrap injector. See #9726.

@tbosch tbosch closed this as completed Jul 7, 2016
@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 Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants