ngneat/spectator

Spectator is forcing to add a transform explicitly if input() has got a transform function

ashutoshpaul opened this issue · 4 comments

Let's say I have a XyzComponent. I have to pass id in route-params to XyzComponent.

Inside XyzComponent:
id = input(undefined, { transform: numberAttribute });

Inside xyz.component.spec.ts file

spectator = createComponent({
    props: {
      id: input(5, { transform: numberAttribute })
    },
  });

as you can see we have to explicitly add { transform: numberAttribute } because the ngneat/spectator v17 doesn't infer transformFn.

Solution that we came up with:

type InferInputSignal<T> = T extends InputSignal<infer K, infer _> ? K : T;
type InferInputSignals<C> = {
    [P in keyof C]+?: InferInputSignal<C[P]>;
};

export interface CustomSpectatorOverrides<C> {
    whenStable?: boolean;
    props?: InferInputSignals<C>;
}

all we did was to add infer _ in InferInputSignal. And we are overriding SpectatorOverrides with CustomSpectatorOverrides, with this we are able to achieve:

spectator = createComponent({ props: { id: 5 }});

this might be a workaround. can you please fix this in appropriate way?

@kfrancois what do you think?

Good catch! Internally, InputSignal is defined as follows:

export declare interface InputSignal<ReadT> extends InputSignalWithTransform<ReadT, ReadT> {
}

Meaning that it extends InputSignalWithTransform, so we need to update InferInputSignal's conditional clause to this type instead.

There is one point where I think some feedback is necessary: InputSignalWithTransform<ReadT, WriteT> has two generic types:

  • ReadT is the type that the value will be transformed to
  • WriteT is the type that we can input into the component

Let's say we have the following component:

class MyComponent {
    disabled = input(false, { transform: booleanAttribute });
}

As disabled is of type InputSignalWithTransform<boolean, unknown>, we can choose to infer either boolean or unknown.

If we decide to infer TWrite:

export type InferInputSignal<T> = T extends InputSignalWithTransform<any, infer K> ? K : T;
export type InferInputSignals<C> = {
    [P in keyof C]+?: InferInputSignal<C[P]>;
};


// { disabled?: unknown }
type MyInputs = InferInputSignals<SidebarPanelEntryComponent>;

If we decide to infer TRead:

export type InferInputSignal<T> = T extends InputSignalWithTransform<infer K, any> ? K : T;
export type InferInputSignals<C> = {
    [P in keyof C]+?: InferInputSignal<C[P]>;
};


// { disabled?: boolean }
type MyInputs = InferInputSignals<SidebarPanelEntryComponent>;

On one hand, it's a much nicer dev experience to have the internal ReadT-type used for autocomplete (ie: we see in our editor that disabled needs to be a boolean).

On the other hand, this makes it harder to test input transforms. Something like booleanAttribute comes from Angular itself so we don't need to test it, but if we define our own transform inside the component, we might want to unit test that.


My personal thoughts are:
I'm in favour of the second option, as I believe it's good practice to define and test input transforms outside of the component (we should not define them as one-off functions inside a component). That might be hard to enforce in this library (as it doesn't care about application best practices, only "testing components").

I considered the option to infer both ReadT and WriteT and just union them, but in reality that would just end up doing something like boolean | unknown which is the same as unknown.

To be honest I'm really busy these days and I don't have time for this :) I trust you to reach a solution and open a PR. Thanks

Disclaimer: I am not an expert with angular testing :) But adding my thoughts ...

I think the solution should be kept to what componentRef.setInput expects as that is used here

So, if setInput expects ReadT - we should follow that. And if it expects WriteT - we should follow that (cause it will be calling the transform function for us).