Skip to content

Commit

Permalink
fix(core): ensure all initializer functions run in an injection conte…
Browse files Browse the repository at this point in the history
…xt (#54761)

Ensures that all of the functions intended to be run in initializers are in an injection context. This is a stop-gap until we have a compiler diagnostic for it.

PR Close #54761
  • Loading branch information
crisbeto authored and atscott committed Mar 12, 2024
1 parent 1f77083 commit 39a50f9
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 34 deletions.
4 changes: 4 additions & 0 deletions packages/core/src/authoring/input/input.ts
Expand Up @@ -6,17 +6,21 @@
* found in the LICENSE file at https://angular.io/license
*/

import {assertInInjectionContext} from '../../di';

import {createInputSignal, InputOptions, InputOptionsWithoutTransform, InputOptionsWithTransform, InputSignal, InputSignalWithTransform} from './input_signal';
import {REQUIRED_UNSET_VALUE} from './input_signal_node';

export function inputFunction<ReadT, WriteT>(
initialValue?: ReadT,
opts?: InputOptions<ReadT, WriteT>): InputSignalWithTransform<ReadT|undefined, WriteT> {
ngDevMode && assertInInjectionContext(input);
return createInputSignal(initialValue, opts);
}

export function inputRequiredFunction<ReadT, WriteT>(opts?: InputOptions<ReadT, WriteT>):
InputSignalWithTransform<ReadT, WriteT> {
ngDevMode && assertInInjectionContext(input);
return createInputSignal(REQUIRED_UNSET_VALUE as never, opts);
}

Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/authoring/queries.ts
Expand Up @@ -6,18 +6,21 @@
* found in the LICENSE file at https://angular.io/license
*/

import {assertInInjectionContext} from '../di';
import {ProviderToken} from '../di/provider_token';
import {createMultiResultQuerySignalFn, createSingleResultOptionalQuerySignalFn, createSingleResultRequiredQuerySignalFn} from '../render3/query_reactive';
import {Signal} from '../render3/reactivity/api';

function viewChildFn<LocatorT, ReadT>(
locator: ProviderToken<LocatorT>|string,
opts?: {read?: ProviderToken<ReadT>}): Signal<ReadT|undefined> {
ngDevMode && assertInInjectionContext(viewChild);
return createSingleResultOptionalQuerySignalFn<ReadT>();
}

function viewChildRequiredFn<LocatorT, ReadT>(
locator: ProviderToken<LocatorT>|string, opts?: {read?: ProviderToken<ReadT>}): Signal<ReadT> {
ngDevMode && assertInInjectionContext(viewChild);
return createSingleResultRequiredQuerySignalFn<ReadT>();
}

Expand Down Expand Up @@ -108,18 +111,21 @@ export function viewChildren<LocatorT, ReadT>(
export function viewChildren<LocatorT, ReadT>(
locator: ProviderToken<LocatorT>|string,
opts?: {read?: ProviderToken<ReadT>}): Signal<ReadonlyArray<ReadT>> {
ngDevMode && assertInInjectionContext(viewChildren);
return createMultiResultQuerySignalFn<ReadT>();
}

export function contentChildFn<LocatorT, ReadT>(
locator: ProviderToken<LocatorT>|string,
opts?: {descendants?: boolean, read?: ProviderToken<ReadT>}): Signal<ReadT|undefined> {
ngDevMode && assertInInjectionContext(contentChild);
return createSingleResultOptionalQuerySignalFn<ReadT>();
}

function contentChildRequiredFn<LocatorT, ReadT>(
locator: ProviderToken<LocatorT>|string,
opts?: {descendants?: boolean, read?: ProviderToken<ReadT>}): Signal<ReadT> {
ngDevMode && assertInInjectionContext(contentChildren);
return createSingleResultRequiredQuerySignalFn<ReadT>();
}

Expand Down
59 changes: 51 additions & 8 deletions packages/core/test/acceptance/authoring/signal_queries_spec.ts
Expand Up @@ -31,10 +31,25 @@ describe('queries as signals', () => {

fixture.detectChanges();
expect(fixture.componentInstance.foundEl()).toBeTrue();
});

// non-required query results are undefined before we run creation mode on the view queries
const appCmpt = new AppComponent();
expect(appCmpt.divEl()).toBeUndefined();
it('should return undefined if optional query is read in the constructor', () => {
let result: {}|undefined = {};

@Component({
standalone: true,
template: `<div #el></div>`,
})
class AppComponent {
divEl = viewChild<ElementRef<HTMLDivElement>>('el');

constructor() {
result = this.divEl();
}
}

TestBed.createComponent(AppComponent);
expect(result).toBeUndefined();
});

it('should query for a required element in a template', () => {
Expand All @@ -55,11 +70,24 @@ describe('queries as signals', () => {

fixture.detectChanges();
expect(fixture.componentInstance.foundEl()).toBeTrue();
});

it('should throw if required query is read in the constructor', () => {
@Component({
standalone: true,
template: `<div #el></div>`,
})
class AppComponent {
divEl = viewChild.required<ElementRef<HTMLDivElement>>('el');

constructor() {
this.divEl();
}
}

// non-required query results are undefined before we run creation mode on the view queries
const appCmpt = new AppComponent();
expect(() => {
appCmpt.divEl();
TestBed.createComponent(AppComponent);
}).toThrowError(/NG0951: Child query result is required but no value is available/);
});

Expand Down Expand Up @@ -96,10 +124,25 @@ describe('queries as signals', () => {
fixture.componentInstance.show = false;
fixture.detectChanges();
expect(fixture.componentInstance.foundEl()).toBe(1);
});

// non-required query results are undefined before we run creation mode on the view queries
const appCmpt = new AppComponent();
expect(appCmpt.divEls().length).toBe(0);
it('should return an empty array when reading children query in the constructor', () => {
let result: readonly ElementRef[]|undefined;

@Component({
standalone: true,
template: `<div #el></div>`,
})
class AppComponent {
divEls = viewChildren<ElementRef<HTMLDivElement>>('el');

constructor() {
result = this.divEls();
}
}

TestBed.createComponent(AppComponent);
expect(result).toEqual([]);
});

it('should return the same array instance when there were no changes in results', () => {
Expand Down
62 changes: 36 additions & 26 deletions packages/core/test/authoring/input_signal_spec.ts
Expand Up @@ -37,51 +37,61 @@ describe('input signal', () => {
});

it('should work with computed expressions', () => {
const signal = input(0);
let computedCount = 0;
const derived = computed(() => (computedCount++, signal() + 1000));
TestBed.runInInjectionContext(() => {
const signal = input(0);
let computedCount = 0;
const derived = computed(() => (computedCount++, signal() + 1000));

const node = signal[SIGNAL];
expect(derived()).toBe(1000);
expect(computedCount).toBe(1);
const node = signal[SIGNAL];
expect(derived()).toBe(1000);
expect(computedCount).toBe(1);

node.applyValueToInputSignal(node, 1);
expect(computedCount).toBe(1);
node.applyValueToInputSignal(node, 1);
expect(computedCount).toBe(1);

expect(derived()).toBe(1001);
expect(computedCount).toBe(2);
expect(derived()).toBe(1001);
expect(computedCount).toBe(2);
});
});

it('should capture transform for later use in framework', () => {
const signal = input(0, {transform: (v: number) => v + 1000});
const node = signal[SIGNAL];
TestBed.runInInjectionContext(() => {
const signal = input(0, {transform: (v: number) => v + 1000});
const node = signal[SIGNAL];

expect(node.transformFn?.(1)).toBe(1001);
expect(node.transformFn?.(1)).toBe(1001);
});
});

it('should throw if there is no value for required inputs', () => {
const signal = input.required();
const node = signal[SIGNAL];
TestBed.runInInjectionContext(() => {
const signal = input.required();
const node = signal[SIGNAL];

expect(() => signal()).toThrowError(/Input is required but no value is available yet\./);
expect(() => signal()).toThrowError(/Input is required but no value is available yet\./);

node.applyValueToInputSignal(node, 1);
expect(signal()).toBe(1);
node.applyValueToInputSignal(node, 1);
expect(signal()).toBe(1);
});
});

it('should throw if a `computed` depends on an uninitialized required input', () => {
const signal = input.required<number>();
const expr = computed(() => signal() + 1000);
const node = signal[SIGNAL];
TestBed.runInInjectionContext(() => {
const signal = input.required<number>();
const expr = computed(() => signal() + 1000);
const node = signal[SIGNAL];

expect(() => expr()).toThrowError(/Input is required but no value is available yet\./);
expect(() => expr()).toThrowError(/Input is required but no value is available yet\./);

node.applyValueToInputSignal(node, 1);
expect(expr()).toBe(1001);
node.applyValueToInputSignal(node, 1);
expect(expr()).toBe(1001);
});
});

it('should have a toString implementation', () => {
const signal = input(0);
expect(signal + '').toBe('[Input Signal: 0]');
TestBed.runInInjectionContext(() => {
const signal = input(0);
expect(signal + '').toBe('[Input Signal: 0]');
});
});
});

0 comments on commit 39a50f9

Please sign in to comment.