2019-01-31 04:38:43 -05:00
|
|
|
/**
|
|
|
|
* @license
|
|
|
|
* Copyright Google Inc. All Rights Reserved.
|
|
|
|
*
|
|
|
|
* Use of this source code is governed by an MIT-style license that can be
|
|
|
|
* found in the LICENSE file at https://angular.io/license
|
|
|
|
*/
|
|
|
|
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
import {Component, Directive, forwardRef, Inject, Injectable, InjectionToken, Injector, NgModule, Optional} from '@angular/core';
|
|
|
|
import {async, inject, TestBed} from '@angular/core/testing';
|
2019-04-16 19:58:44 -04:00
|
|
|
import {By} from '@angular/platform-browser';
|
2019-05-18 04:10:19 -04:00
|
|
|
import {expect} from '@angular/platform-browser/testing/src/matchers';
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
import {modifiedInIvy, onlyInIvy} from '@angular/private/testing';
|
2019-01-31 04:38:43 -05:00
|
|
|
|
|
|
|
describe('providers', () => {
|
2019-04-16 19:58:44 -04:00
|
|
|
describe('inheritance', () => {
|
|
|
|
it('should NOT inherit providers', () => {
|
|
|
|
const SOME_DIRS = new InjectionToken('someDirs');
|
|
|
|
|
|
|
|
@Directive({
|
|
|
|
selector: '[super-dir]',
|
|
|
|
providers: [{provide: SOME_DIRS, useClass: SuperDirective, multi: true}]
|
|
|
|
})
|
|
|
|
class SuperDirective {
|
|
|
|
}
|
|
|
|
|
|
|
|
@Directive({
|
|
|
|
selector: '[sub-dir]',
|
|
|
|
providers: [{provide: SOME_DIRS, useClass: SubDirective, multi: true}]
|
|
|
|
})
|
|
|
|
class SubDirective extends SuperDirective {
|
|
|
|
}
|
|
|
|
|
|
|
|
@Directive({selector: '[other-dir]'})
|
|
|
|
class OtherDirective {
|
|
|
|
constructor(@Inject(SOME_DIRS) public dirs: any) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
@Component({selector: 'app-comp', template: `<div other-dir sub-dir></div>`})
|
|
|
|
class App {
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule(
|
|
|
|
{declarations: [SuperDirective, SubDirective, OtherDirective, App]});
|
|
|
|
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
|
|
|
|
const otherDir = fixture.debugElement.query(By.css('div')).injector.get(OtherDirective);
|
|
|
|
expect(otherDir.dirs.length).toEqual(1);
|
|
|
|
expect(otherDir.dirs[0] instanceof SubDirective).toBe(true);
|
|
|
|
});
|
|
|
|
});
|
|
|
|
|
2019-01-31 04:38:43 -05:00
|
|
|
describe('lifecycles', () => {
|
|
|
|
it('should inherit ngOnDestroy hooks on providers', () => {
|
|
|
|
const logs: string[] = [];
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class SuperInjectableWithDestroyHook {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
ngOnDestroy() {
|
|
|
|
logs.push('OnDestroy');
|
|
|
|
}
|
2019-01-31 04:38:43 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class SubInjectableWithDestroyHook extends SuperInjectableWithDestroyHook {
|
|
|
|
}
|
|
|
|
|
|
|
|
@Component({template: '', providers: [SubInjectableWithDestroyHook]})
|
|
|
|
class App {
|
|
|
|
constructor(foo: SubInjectableWithDestroyHook) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(logs).toEqual(['OnDestroy']);
|
|
|
|
});
|
|
|
|
|
|
|
|
it('should not call ngOnDestroy for providers that have not been requested', () => {
|
|
|
|
const logs: string[] = [];
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class InjectableWithDestroyHook {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
ngOnDestroy() {
|
|
|
|
logs.push('OnDestroy');
|
|
|
|
}
|
2019-01-31 04:38:43 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Component({template: '', providers: [InjectableWithDestroyHook]})
|
|
|
|
class App {
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(logs).toEqual([]);
|
|
|
|
});
|
|
|
|
|
|
|
|
it('should only call ngOnDestroy once for multiple instances', () => {
|
|
|
|
const logs: string[] = [];
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class InjectableWithDestroyHook {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
ngOnDestroy() {
|
|
|
|
logs.push('OnDestroy');
|
|
|
|
}
|
2019-01-31 04:38:43 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Component({selector: 'my-cmp', template: ''})
|
|
|
|
class MyComponent {
|
|
|
|
constructor(foo: InjectableWithDestroyHook) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
template: `
|
|
|
|
<my-cmp></my-cmp>
|
|
|
|
<my-cmp></my-cmp>
|
|
|
|
`,
|
|
|
|
providers: [InjectableWithDestroyHook]
|
|
|
|
})
|
|
|
|
class App {
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App, MyComponent]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(logs).toEqual(['OnDestroy']);
|
|
|
|
});
|
|
|
|
|
|
|
|
it('should call ngOnDestroy when providing same token via useClass', () => {
|
|
|
|
const logs: string[] = [];
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class InjectableWithDestroyHook {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
ngOnDestroy() {
|
|
|
|
logs.push('OnDestroy');
|
|
|
|
}
|
2019-01-31 04:38:43 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
template: '',
|
|
|
|
providers: [{provide: InjectableWithDestroyHook, useClass: InjectableWithDestroyHook}]
|
|
|
|
})
|
|
|
|
class App {
|
|
|
|
constructor(foo: InjectableWithDestroyHook) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(logs).toEqual(['OnDestroy']);
|
|
|
|
});
|
|
|
|
|
|
|
|
onlyInIvy('Destroy hook of useClass provider is invoked correctly')
|
|
|
|
.it('should only call ngOnDestroy of value when providing via useClass', () => {
|
|
|
|
const logs: string[] = [];
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class InjectableWithDestroyHookToken {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
ngOnDestroy() {
|
|
|
|
logs.push('OnDestroy Token');
|
|
|
|
}
|
2019-01-31 04:38:43 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class InjectableWithDestroyHookValue {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
ngOnDestroy() {
|
|
|
|
logs.push('OnDestroy Value');
|
|
|
|
}
|
2019-01-31 04:38:43 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
template: '',
|
|
|
|
providers: [
|
|
|
|
{provide: InjectableWithDestroyHookToken, useClass: InjectableWithDestroyHookValue}
|
|
|
|
]
|
|
|
|
})
|
|
|
|
class App {
|
|
|
|
constructor(foo: InjectableWithDestroyHookToken) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(logs).toEqual(['OnDestroy Value']);
|
|
|
|
});
|
|
|
|
|
|
|
|
it('should only call ngOnDestroy of value when providing via useExisting', () => {
|
|
|
|
const logs: string[] = [];
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class InjectableWithDestroyHookToken {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
ngOnDestroy() {
|
|
|
|
logs.push('OnDestroy Token');
|
|
|
|
}
|
2019-01-31 04:38:43 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class InjectableWithDestroyHookExisting {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
ngOnDestroy() {
|
|
|
|
logs.push('OnDestroy Existing');
|
|
|
|
}
|
2019-01-31 04:38:43 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
template: '',
|
|
|
|
providers: [
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
InjectableWithDestroyHookExisting,
|
|
|
|
{provide: InjectableWithDestroyHookToken, useExisting: InjectableWithDestroyHookExisting}
|
2019-01-31 04:38:43 -05:00
|
|
|
]
|
|
|
|
})
|
|
|
|
class App {
|
|
|
|
constructor(foo1: InjectableWithDestroyHookExisting, foo2: InjectableWithDestroyHookToken) {
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(logs).toEqual(['OnDestroy Existing']);
|
|
|
|
});
|
|
|
|
|
2020-02-14 12:54:10 -05:00
|
|
|
it('should invoke ngOnDestroy with the correct context when providing a type provider multiple times on the same node',
|
|
|
|
() => {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
const resolvedServices: (DestroyService|undefined)[] = [];
|
|
|
|
const destroyContexts: (DestroyService|undefined)[] = [];
|
2020-02-14 12:54:10 -05:00
|
|
|
let parentService: DestroyService|undefined;
|
|
|
|
let childService: DestroyService|undefined;
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class DestroyService {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
constructor() {
|
|
|
|
resolvedServices.push(this);
|
|
|
|
}
|
|
|
|
ngOnDestroy() {
|
|
|
|
destroyContexts.push(this);
|
|
|
|
}
|
2020-02-14 12:54:10 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Directive({selector: '[dir-one]', providers: [DestroyService]})
|
|
|
|
class DirOne {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
constructor(service: DestroyService) {
|
|
|
|
childService = service;
|
|
|
|
}
|
2020-02-14 12:54:10 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Directive({selector: '[dir-two]', providers: [DestroyService]})
|
|
|
|
class DirTwo {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
constructor(service: DestroyService) {
|
|
|
|
childService = service;
|
|
|
|
}
|
2020-02-14 12:54:10 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Component({template: '<div dir-one dir-two></div>', providers: [DestroyService]})
|
|
|
|
class App {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
constructor(service: DestroyService) {
|
|
|
|
parentService = service;
|
|
|
|
}
|
2020-02-14 12:54:10 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App, DirOne, DirTwo]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(parentService).toBeDefined();
|
|
|
|
expect(childService).toBeDefined();
|
|
|
|
expect(parentService).not.toBe(childService);
|
|
|
|
expect(resolvedServices).toEqual([parentService, childService]);
|
|
|
|
expect(destroyContexts).toEqual([parentService, childService]);
|
|
|
|
});
|
|
|
|
|
|
|
|
onlyInIvy('Destroy hook of useClass provider is invoked correctly')
|
|
|
|
.it('should invoke ngOnDestroy with the correct context when providing a class provider multiple times on the same node',
|
|
|
|
() => {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
const resolvedServices: (DestroyService|undefined)[] = [];
|
|
|
|
const destroyContexts: (DestroyService|undefined)[] = [];
|
2020-02-14 12:54:10 -05:00
|
|
|
const token = new InjectionToken<any>('token');
|
|
|
|
let parentService: DestroyService|undefined;
|
|
|
|
let childService: DestroyService|undefined;
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class DestroyService {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
constructor() {
|
|
|
|
resolvedServices.push(this);
|
|
|
|
}
|
|
|
|
ngOnDestroy() {
|
|
|
|
destroyContexts.push(this);
|
|
|
|
}
|
2020-02-14 12:54:10 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Directive(
|
|
|
|
{selector: '[dir-one]', providers: [{provide: token, useClass: DestroyService}]})
|
|
|
|
class DirOne {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
constructor(@Inject(token) service: DestroyService) {
|
|
|
|
childService = service;
|
|
|
|
}
|
2020-02-14 12:54:10 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Directive(
|
|
|
|
{selector: '[dir-two]', providers: [{provide: token, useClass: DestroyService}]})
|
|
|
|
class DirTwo {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
constructor(@Inject(token) service: DestroyService) {
|
|
|
|
childService = service;
|
|
|
|
}
|
2020-02-14 12:54:10 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
template: '<div dir-one dir-two></div>',
|
|
|
|
providers: [{provide: token, useClass: DestroyService}]
|
|
|
|
})
|
|
|
|
class App {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
constructor(@Inject(token) service: DestroyService) {
|
|
|
|
parentService = service;
|
|
|
|
}
|
2020-02-14 12:54:10 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App, DirOne, DirTwo]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(parentService).toBeDefined();
|
|
|
|
expect(childService).toBeDefined();
|
|
|
|
expect(parentService).not.toBe(childService);
|
|
|
|
expect(resolvedServices).toEqual([parentService, childService]);
|
|
|
|
expect(destroyContexts).toEqual([parentService, childService]);
|
|
|
|
});
|
|
|
|
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
|
2020-02-14 12:54:10 -05:00
|
|
|
onlyInIvy('ngOnDestroy hooks for multi providers were not supported in ViewEngine')
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
.describe('ngOnDestroy on multi providers', () => {
|
|
|
|
it('should invoke ngOnDestroy on multi providers with the correct context', () => {
|
|
|
|
const destroyCalls: any[] = [];
|
|
|
|
const SERVICES = new InjectionToken<any>('SERVICES');
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class DestroyService {
|
|
|
|
ngOnDestroy() {
|
|
|
|
destroyCalls.push(this);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class OtherDestroyService {
|
|
|
|
ngOnDestroy() {
|
|
|
|
destroyCalls.push(this);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
template: '<div></div>',
|
|
|
|
providers: [
|
|
|
|
{provide: SERVICES, useClass: DestroyService, multi: true},
|
|
|
|
{provide: SERVICES, useClass: OtherDestroyService, multi: true},
|
|
|
|
]
|
|
|
|
})
|
|
|
|
class App {
|
|
|
|
constructor(@Inject(SERVICES) s: any) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(destroyCalls).toEqual([
|
|
|
|
jasmine.any(DestroyService), jasmine.any(OtherDestroyService)
|
|
|
|
]);
|
|
|
|
});
|
|
|
|
|
|
|
|
it('should invoke destroy hooks on multi providers with the correct context, if only some have a destroy hook',
|
|
|
|
() => {
|
|
|
|
const destroyCalls: any[] = [];
|
|
|
|
const SERVICES = new InjectionToken<any>('SERVICES');
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class Service1 {
|
|
|
|
}
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class Service2 {
|
|
|
|
ngOnDestroy() {
|
|
|
|
destroyCalls.push(this);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class Service3 {
|
|
|
|
}
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class Service4 {
|
|
|
|
ngOnDestroy() {
|
|
|
|
destroyCalls.push(this);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
template: '<div></div>',
|
|
|
|
providers: [
|
|
|
|
{provide: SERVICES, useClass: Service1, multi: true},
|
|
|
|
{provide: SERVICES, useClass: Service2, multi: true},
|
|
|
|
{provide: SERVICES, useClass: Service3, multi: true},
|
|
|
|
{provide: SERVICES, useClass: Service4, multi: true},
|
|
|
|
]
|
|
|
|
})
|
|
|
|
class App {
|
|
|
|
constructor(@Inject(SERVICES) s: any) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(destroyCalls).toEqual([jasmine.any(Service2), jasmine.any(Service4)]);
|
|
|
|
});
|
|
|
|
|
|
|
|
it('should not invoke ngOnDestroy on multi providers created via useFactory', () => {
|
|
|
|
let destroyCalls = 0;
|
|
|
|
const SERVICES = new InjectionToken<any>('SERVICES');
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class DestroyService {
|
|
|
|
ngOnDestroy() {
|
|
|
|
destroyCalls++;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class OtherDestroyService {
|
|
|
|
ngOnDestroy() {
|
|
|
|
destroyCalls++;
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
template: '<div></div>',
|
|
|
|
providers: [
|
|
|
|
{provide: SERVICES, useFactory: () => new DestroyService(), multi: true},
|
|
|
|
{provide: SERVICES, useFactory: () => new OtherDestroyService(), multi: true},
|
|
|
|
]
|
|
|
|
})
|
|
|
|
class App {
|
|
|
|
constructor(@Inject(SERVICES) s: any) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
|
|
|
expect(destroyCalls).toBe(0);
|
|
|
|
});
|
|
|
|
});
|
|
|
|
|
|
|
|
modifiedInIvy('ViewEngine did not support destroy hooks on multi providers')
|
2020-02-14 12:54:10 -05:00
|
|
|
.it('should not invoke ngOnDestroy on multi providers', () => {
|
|
|
|
let destroyCalls = 0;
|
|
|
|
const SERVICES = new InjectionToken<any>('SERVICES');
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class DestroyService {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
ngOnDestroy() {
|
|
|
|
destroyCalls++;
|
|
|
|
}
|
2020-02-14 12:54:10 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class OtherDestroyService {
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
ngOnDestroy() {
|
|
|
|
destroyCalls++;
|
|
|
|
}
|
2020-02-14 12:54:10 -05:00
|
|
|
}
|
|
|
|
|
|
|
|
@Component({
|
|
|
|
template: '<div></div>',
|
|
|
|
providers: [
|
|
|
|
{provide: SERVICES, useClass: DestroyService, multi: true},
|
|
|
|
{provide: SERVICES, useClass: OtherDestroyService, multi: true},
|
|
|
|
]
|
|
|
|
})
|
|
|
|
class App {
|
|
|
|
constructor(@Inject(SERVICES) s: any) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({declarations: [App]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
fixture.destroy();
|
|
|
|
|
fix(core): ngOnDestroy on multi providers called with incorrect context (#35840)
Currently destroy hooks are stored in memory as `[1, hook, 5, hook]` where
the numbers represent the index at which to find the context and `hook` is
the function to be invoked. This breaks down for `multi` providers,
because the value at the index will be an array of providers, resulting in
the hook being invoked with an array of all the multi provider values,
rather than the provider that was destroyed. In ViewEngine `ngOnDestroy`
wasn't being called for `multi` providers at all.
These changes fix the issue by changing the structure of the destroy hooks to `[1, hook, 5, [0, hook, 3, hook]]` where the indexes inside the inner array point to the provider inside of the multi provider array. Note that this is slightly different from the original design which called for the structure to be `[1, hook, 5, [hook, hook]`, because in the process of implementing it, I realized that we wouldn't get passing the correct context if only some of the `multi` providers have `ngOnDestroy` and others don't.
I've run the newly-added `view_destroy_hooks` benchmark against these changes and compared it to master. The difference seems to be insignificant (between 1% and 2% slower).
Fixes #35231.
PR Close #35840
2020-03-03 15:05:26 -05:00
|
|
|
expect(destroyCalls).toBe(0);
|
2020-02-14 12:54:10 -05:00
|
|
|
});
|
2019-01-31 04:38:43 -05:00
|
|
|
});
|
2019-04-16 19:58:44 -04:00
|
|
|
|
|
|
|
describe('components and directives', () => {
|
|
|
|
class MyService {
|
|
|
|
value = 'some value';
|
|
|
|
}
|
|
|
|
|
|
|
|
@Component({selector: 'my-comp', template: ``})
|
|
|
|
class MyComp {
|
|
|
|
constructor(public svc: MyService) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
@Directive({selector: '[some-dir]'})
|
|
|
|
class MyDir {
|
|
|
|
constructor(public svc: MyService) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
it('should support providing components in tests without @Injectable', () => {
|
|
|
|
@Component({selector: 'test-comp', template: '<my-comp></my-comp>'})
|
|
|
|
class TestComp {
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({
|
|
|
|
declarations: [TestComp, MyComp],
|
|
|
|
// providing MyComp is unnecessary but it shouldn't throw
|
|
|
|
providers: [MyComp, MyService],
|
|
|
|
});
|
|
|
|
|
|
|
|
const fixture = TestBed.createComponent(TestComp);
|
|
|
|
const myCompInstance = fixture.debugElement.query(By.css('my-comp')).injector.get(MyComp);
|
|
|
|
expect(myCompInstance.svc.value).toEqual('some value');
|
|
|
|
});
|
|
|
|
|
|
|
|
it('should support providing directives in tests without @Injectable', () => {
|
|
|
|
@Component({selector: 'test-comp', template: '<div some-dir></div>'})
|
|
|
|
class TestComp {
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({
|
|
|
|
declarations: [TestComp, MyDir],
|
|
|
|
// providing MyDir is unnecessary but it shouldn't throw
|
|
|
|
providers: [MyDir, MyService],
|
|
|
|
});
|
|
|
|
|
|
|
|
const fixture = TestBed.createComponent(TestComp);
|
|
|
|
const myCompInstance = fixture.debugElement.query(By.css('div')).injector.get(MyDir);
|
|
|
|
expect(myCompInstance.svc.value).toEqual('some value');
|
|
|
|
});
|
2019-04-16 22:03:40 -04:00
|
|
|
|
|
|
|
describe('injection without bootstrapping', () => {
|
|
|
|
beforeEach(() => {
|
|
|
|
TestBed.configureTestingModule({declarations: [MyComp], providers: [MyComp, MyService]});
|
|
|
|
});
|
|
|
|
|
|
|
|
it('should support injecting without bootstrapping',
|
|
|
|
async(inject([MyComp, MyService], (comp: MyComp, service: MyService) => {
|
|
|
|
expect(comp.svc.value).toEqual('some value');
|
|
|
|
})));
|
|
|
|
});
|
2019-04-16 19:58:44 -04:00
|
|
|
});
|
2019-04-29 18:21:49 -04:00
|
|
|
|
|
|
|
describe('forward refs', () => {
|
|
|
|
it('should support forward refs in provider deps', () => {
|
|
|
|
class MyService {
|
|
|
|
constructor(public dep: {value: string}) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
class OtherService {
|
|
|
|
value = 'one';
|
|
|
|
}
|
|
|
|
|
|
|
|
@Component({selector: 'app-comp', template: ``})
|
|
|
|
class AppComp {
|
|
|
|
constructor(public myService: MyService) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
@NgModule({
|
|
|
|
providers: [
|
|
|
|
OtherService, {
|
|
|
|
provide: MyService,
|
|
|
|
useFactory: (dep: {value: string}) => new MyService(dep),
|
|
|
|
deps: [forwardRef(() => OtherService)]
|
|
|
|
}
|
|
|
|
],
|
|
|
|
declarations: [AppComp]
|
|
|
|
})
|
|
|
|
class MyModule {
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule({imports: [MyModule]});
|
|
|
|
|
|
|
|
const fixture = TestBed.createComponent(AppComp);
|
|
|
|
expect(fixture.componentInstance.myService.dep.value).toBe('one');
|
|
|
|
});
|
|
|
|
|
2019-05-18 04:10:19 -04:00
|
|
|
it('should support forward refs in useClass when impl version is also provided', () => {
|
|
|
|
@Injectable({providedIn: 'root', useClass: forwardRef(() => SomeProviderImpl)})
|
|
|
|
abstract class SomeProvider {
|
|
|
|
}
|
|
|
|
|
|
|
|
@Injectable()
|
|
|
|
class SomeProviderImpl extends SomeProvider {
|
|
|
|
}
|
|
|
|
|
|
|
|
@Component({selector: 'my-app', template: ''})
|
|
|
|
class App {
|
|
|
|
constructor(public foo: SomeProvider) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
TestBed.configureTestingModule(
|
|
|
|
{declarations: [App], providers: [{provide: SomeProvider, useClass: SomeProviderImpl}]});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
|
|
|
|
|
|
|
expect(fixture.componentInstance.foo).toBeAnInstanceOf(SomeProviderImpl);
|
|
|
|
});
|
|
|
|
|
|
|
|
|
2020-02-22 05:18:33 -05:00
|
|
|
it('should support forward refs in useClass when token is provided', () => {
|
|
|
|
@Injectable({providedIn: 'root'})
|
|
|
|
abstract class SomeProvider {
|
|
|
|
}
|
2019-05-18 04:10:19 -04:00
|
|
|
|
2020-02-22 05:18:33 -05:00
|
|
|
@Injectable()
|
|
|
|
class SomeProviderImpl extends SomeProvider {
|
|
|
|
}
|
2019-05-18 04:10:19 -04:00
|
|
|
|
2020-02-22 05:18:33 -05:00
|
|
|
@Component({selector: 'my-app', template: ''})
|
|
|
|
class App {
|
|
|
|
constructor(public foo: SomeProvider) {}
|
|
|
|
}
|
2019-05-18 04:10:19 -04:00
|
|
|
|
2020-02-22 05:18:33 -05:00
|
|
|
TestBed.configureTestingModule({
|
|
|
|
declarations: [App],
|
|
|
|
providers: [{provide: SomeProvider, useClass: forwardRef(() => SomeProviderImpl)}]
|
|
|
|
});
|
|
|
|
const fixture = TestBed.createComponent(App);
|
|
|
|
fixture.detectChanges();
|
2019-05-18 04:10:19 -04:00
|
|
|
|
2020-02-22 05:18:33 -05:00
|
|
|
expect(fixture.componentInstance.foo).toBeAnInstanceOf(SomeProviderImpl);
|
|
|
|
});
|
2019-04-29 18:21:49 -04:00
|
|
|
});
|
|
|
|
|
2019-04-30 19:41:18 -04:00
|
|
|
describe('flags', () => {
|
|
|
|
class MyService {
|
|
|
|
constructor(public value: OtherService|null) {}
|
|
|
|
}
|
|
|
|
|
|
|
|
class OtherService {}
|
|
|
|
|
|
|
|
it('should support Optional flag in deps', () => {
|
|
|
|
const injector =
|
|
|
|
Injector.create([{provide: MyService, deps: [[new Optional(), OtherService]]}]);
|
|
|
|
|
|
|
|
expect(injector.get(MyService).value).toBe(null);
|
|
|
|
});
|
|
|
|
|
|
|
|
it('should support Optional flag in deps without instantiating it', () => {
|
|
|
|
const injector = Injector.create([{provide: MyService, deps: [[Optional, OtherService]]}]);
|
|
|
|
|
|
|
|
expect(injector.get(MyService).value).toBe(null);
|
|
|
|
});
|
|
|
|
});
|
2019-01-31 04:38:43 -05:00
|
|
|
});
|