Navigation Menu

Skip to content

Commit

Permalink
fix(ivy): reorder provider type checks to align with VE (#34433)
Browse files Browse the repository at this point in the history
The ordering matters because we don't currently throw if multiple
configurations are provided (i.e. provider has *both* useExisting and
useFactory). We should actually throw an error in this case, but to
avoid another breaking change in v9, this PR simply aligns the Ivy
behavior with ViewEngine.

PR Close #34433
  • Loading branch information
atscott authored and kara committed Dec 16, 2019
1 parent 6607fb4 commit 7916b1e
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 2 deletions.
4 changes: 2 additions & 2 deletions packages/core/src/di/r3_injector.ts
Expand Up @@ -482,10 +482,10 @@ export function providerToFactory(
} else { } else {
if (isValueProvider(provider)) { if (isValueProvider(provider)) {
factory = () => resolveForwardRef(provider.useValue); factory = () => resolveForwardRef(provider.useValue);
} else if (isExistingProvider(provider)) {
factory = () => ɵɵinject(resolveForwardRef(provider.useExisting));
} else if (isFactoryProvider(provider)) { } else if (isFactoryProvider(provider)) {
factory = () => provider.useFactory(...injectArgs(provider.deps || [])); factory = () => provider.useFactory(...injectArgs(provider.deps || []));
} else if (isExistingProvider(provider)) {
factory = () => ɵɵinject(resolveForwardRef(provider.useExisting));
} else { } else {
const classRef = resolveForwardRef( const classRef = resolveForwardRef(
provider && provider &&
Expand Down
32 changes: 32 additions & 0 deletions packages/core/test/acceptance/di_spec.ts
Expand Up @@ -1901,4 +1901,36 @@ describe('di', () => {


expect(fixture.nativeElement.textContent.trim()).toBe('2 (transformed) items'); expect(fixture.nativeElement.textContent.trim()).toBe('2 (transformed) items');
}); });

// TODO: https://angular-team.atlassian.net/browse/FW-1779
it('should prioritize useFactory over useExisting', () => {
abstract class Base {}
@Directive({selector: '[dirA]'})
class DirA implements Base {
}
@Directive({selector: '[dirB]'})
class DirB implements Base {
}

const PROVIDER = {provide: Base, useExisting: DirA, useFactory: () => new DirB()};

@Component({selector: 'child', template: '', providers: [PROVIDER]})
class Child {
constructor(readonly base: Base) {}
}

@Component({template: `<div dirA> <child></child> </div>`})
class App {
@ViewChild(DirA) dirA !: DirA;
@ViewChild(Child) child !: Child;
}

const fixture = TestBed.configureTestingModule({declarations: [DirA, DirB, App, Child]})
.createComponent(App);
fixture.detectChanges();
expect(fixture.componentInstance.dirA)
.not.toEqual(
fixture.componentInstance.child.base,
'should not get dirA from parent, but create new dirB from the useFactory provider');
});
}); });

0 comments on commit 7916b1e

Please sign in to comment.