Open cusher opened 9 months ago
This is hardly achievable since the input can be aliased an this is not represented in the typings. This is the same thing for required inputs, it's not represented in the type that the decorator has required: true
(remember decorators have no effect on typings).
Ah. Is the AOT compiler is only able to catch and report problems in templates, and not in the TS code? I was hoping that since these sorts of errors bubble up in templates, that it would also be feasible to bubble them up in TS. If that's not the case, then I see how that's a problem...
That being said, the current method of setInput
is also not very dependable at scale, IMO. It is very reliant on testing to avoid bugs that "should" be caught by type checking, and even if you you do have extensive tests it breaks down if you make a change without properly updating tests, or do update tests but fail to update every invocation outside of tests. (When recently updating a project code-base to move from string keys to properties w/ type inference in another aspect, there were a shocking number of bugs that had crept in over the years for this exact reason that we discovered all at once.) So I'm reluctant to ever use something like that without strong guardrails in place.
If it's just not feasible to add something more rigid, then it is what it is, we'll probably just continue using the props of the component instance with workarounds for our own use cases. Thanks for the response.
I'm currently facing the same issue. setInput()
offers almost no guarantees, other than runtime exception for incorrect property: https://stackblitz.com/edit/stackblitz-starters-6zmbjw?file=src%2Fmain.ts
If nothing else, some way to make input required would be nice.
@BojanKogoj @cusher here is my take on type safe component inputs https://stackblitz.com/edit/stackblitz-starters-vlhdvr?file=src%2Fmain.ts
@SebastianPodgajny Really cool, impressive to see a purely TS solution!
It would be great if there was a way to get this to work with existing components using regular @Input
s without needing to declare a separate NgInputs
field... but it's not clear to me how that could be accomplished (or if it's even feasible).
Maybe there's a way to leverage TS decorators for that? It seems at least somewhat plausible to me that the @Component
decorator could "generate" extra type information for each of the @Input
s, then a similar scheme to what you provided could be used from there on... but easier said than done.
Can it be a specific signal inputs feature? With the new input function both typings and required can be inferred I believe. This issue seems really important and the new changes complement it.
I did some type gymnastics to try and type signal inputs correctly for this issue to be feasible. The two main ideas are type safety in setInput
and an option to set inputs (especially required) at createComponent
call site. For brevity (it is possible though) I omitted model
and just handled input
. The idea is looking just on signal based input/models and not on decorators.
The end result should be:
const cp = this.vcr.createComponent(Counter, {
signalInputs: {
a: 1,
c: 3
}
});
cp.setSignalInput('a', 2);
cp.setSignalInput('c', 2);
// Hopefully possible too
cp.setSignalInputs({
a: 2,
c: 2,
});
The important parts are:
setSignalInputs
full type safety - meaning a
is the property name or alias if existssignalInputs
is required and will be either an empty object with optional properties if no required inputs or an object with required inputs as required props. Here too aliases will work correctly in a type safe way.The interface InputSignalWithTransform
is the base interface which will change to inherit from a "real" base interface:
interface BaseInputSignal<
ReadT,
WriteT,
RequiredT extends boolean,
AliasT extends string
> {
[SIGNAL]: InputSignalNode<ReadT, WriteT>;
[ɵINPUT_SIGNAL_BRAND_READ_TYPE]: ReadT;
[ɵINPUT_SIGNAL_BRAND_WRITE_TYPE]: WriteT;
[ɵINPUT_SIGNAL_REQUIRED]: RequiredT;
[ɵINPUT_SIGNAL_ALIAS]: AliasT;
}
This allows InputFunction
to be a fully type safe interface in terms of required and alias.
The interface InputOptions
will need to know the new alias type too:
interface InputOptions<ReadT, WriteT, AliasT extends string> {
alias?: AliasT;
transform?: (v: WriteT) => ReadT;
}
The usage will be almost similar to now but will return a more specific type:
// BaseInputSignal<number | undefined, number | undefined, false, never>
input<number>();
// BaseInputSignal<number, number, false, never>
input(2);
// BaseInputSignal<string, number, false, never>
input('2', { transform: (n: number) => n.toString() });
// BaseInputSignal<string, number, false, 'hello'>
input('2', {
transform: (n: number) => n.toString(),
alias: 'hello',
});
// BaseInputSignal<number, number, false, 'hello'>
input(2, { alias: 'hello' });
// BaseInputSignal<number, number, true, never>
input.required<number>();
// BaseInputSignal<string, number, true, never>
input.required({
transform: (n: number) => n.toString(),
});
// BaseInputSignal<string, number, true, 'hello'>
input.required({
transform: (n: number) => n.toString(),
alias: 'hello',
});
The only missing declaration is required input with alias. Currently it is declared as input.required<number>({alias: 'hello'})
but this is not possible with typed alias and will require: input.required<number, 'hello'>({alias: 'hello'})
which isn't a good idea.
For this case I propose a new option just for this case called type
which will use a simple function const type = <T>(): T => undefined as T;
and will be used like this:
// BaseInputSignal<number, number, true, 'hello'>
input.required({ type: type<number>(), alias: 'hello' });
The actual InputFunction
will look like this:
interface InputFunction {
<ReadT>(): BaseInputSignal<
ReadT | undefined,
ReadT | undefined,
false,
never
>;
<ReadT, TAlias extends string = never>(
initialValue: ReadT,
opts?: InputOptionsWithoutTransform<ReadT, TAlias>
): BaseInputSignal<ReadT, ReadT, false, TAlias>;
<ReadT, WriteT, TAlias extends string = never>(
initialValue: ReadT,
opts: InputOptionsWithTransform<ReadT, WriteT, TAlias>
): BaseInputSignal<ReadT, WriteT, false, TAlias>;
required: {
<ReadT, TAlias extends string = never>(
opts?: InputOptionsWithoutTransform<ReadT, TAlias> & {
type?: ReadT;
}
): BaseInputSignal<ReadT, ReadT, true, TAlias>;
<ReadT, WriteT, TAlias extends string = never>(
opts: InputOptionsWithTransform<ReadT, WriteT, TAlias>
): BaseInputSignal<ReadT, WriteT, true, TAlias>;
};
}
At this point all the parts are ready and we can type the new setSignalInput
and signalInputs
options.
// Helpers for the BaseInputSignal
type GeneralBaseInputSignal = BaseInputSignal<any, any, any, any>;
type BaseInputSignalAlias<T> = T extends BaseInputSignal<any, any, any, infer T>
? T
: never;
type BaseInputSignalRequired<T> = T extends BaseInputSignal<
any,
any,
infer T,
any
>
? T
: never;
type BaseInputSignalWrite<T> = T extends BaseInputSignal<any, infer T, any, any>
? T
: never;
// Properties info extraction (mainly alias and required related)
type AllInputSignalPropNames<CompT> = {
[P in keyof CompT]: CompT[P] extends GeneralBaseInputSignal ? P : never;
}[keyof CompT];
type AllInputSignalAliasToPropNameMap<CompT> = {
[P in AllInputSignalPropNames<CompT> as BaseInputSignalAlias<
CompT[P]
> extends never
? P
: BaseInputSignalAlias<CompT[P]>]: P;
};
type InputSignalAliasByRequired<CompT, RequiredT extends boolean> = {
[AliasT in keyof AllInputSignalAliasToPropNameMap<CompT>]: BaseInputSignalRequired<
CompT[AllInputSignalAliasToPropNameMap<CompT>[AliasT] & keyof CompT]
> extends RequiredT
? AliasT
: never;
}[keyof AllInputSignalAliasToPropNameMap<CompT>];
// Won't accept comp as will be implicit due to being in ComponentRef
const setSignalInput = <
CompT,
AliasT extends keyof AllInputSignalAliasToPropNameMap<CompT>
>(
comp: Type<CompT>,
key: AliasT,
value: BaseInputSignalWrite<
CompT[AllInputSignalAliasToPropNameMap<CompT>[AliasT] & keyof CompT]
>
) => undefined;
// Will be the type of the new property inside createComponent
type SignalInputs<CompT> = {
[P in InputSignalAliasByRequired<CompT, true>]: BaseInputSignalWrite<
CompT[AllInputSignalAliasToPropNameMap<CompT>[P] & keyof CompT]
>;
} & {
[P in InputSignalAliasByRequired<CompT, false>]?: BaseInputSignalWrite<
CompT[AllInputSignalAliasToPropNameMap<CompT>[P] & keyof CompT]
>;
};
With this we have fully type safe dynamic components with signal inputs 😀 @dylhunn I hope this can help the idea being implemented
@JeanMeche @pkozlowski-opensource Any thought on the above proposal or something similar? Input metadata integrated into the typing seems the way to go here.
I'm currently facing the same issue.
setInput()
offers almost no guarantees, other than runtime exception for incorrect property
It's not even a runtime exception. Just a console error, so we can't even capture it. Would be nice if setInput()
at least returned a boolean indicating whether the operation was successful or not.
Which @angular/* package(s) are relevant/related to the feature request?
core
Description
When a component is created via the
createComponent
method onViewContainerRef
, there are two ways inputs can currently be set:setInput
method on theComponentRef
. This seems to be the recommended method, however it comes with its own set of issues: a. Thename
can be anystring
, and thus is not guaranteed to be the name of an actual input. b. Thevalue
type isunknown
, so even if you are always using the right names, there is no way to have any sort of type-checking on the value itself.Additionally, for both options, because the inputs are being set one-by-one, there is no way to check for missing
required
inputs.As a result, setting inputs on components created with
createComponent
is quite a bit riskier than using components in templates. It would be great if the API for creating component instances dynamically in TypeScript felt just as robust as instantiating a component inside of a template. Additionally, improvements here this could trickle down to provide a better interface for setting inputs in other use cases e.g. Dialog creation in Angular Components.Proposed solution
Add an
inputs
field to the options parameter ofcreateComponent
which allows passing in initial values for each input on the component. Ideally, this should:required
flag, so if an required input is missing, an error is passed along.Alternatives considered
setInput
onComponentRef
which allows setting inputs in a type safe manner. This would be a more flexible option than the proposed solution, but has no ability to check if required inputs have been set.