ngneat / spectator

🦊 🚀 A Powerful Tool to Simplify Your Angular Tests
https://ngneat.github.io/spectator
MIT License
2.08k stars 177 forks source link

Cannot pass props which are not input() type in createComponent() #650

Open ashutoshpaul opened 7 months ago

ashutoshpaul commented 7 months ago

For better clarity lets assume theirs a XyzComponent

@Component({ ... })
export class XyzComponent {
  name = input();
  age: number;
  id: number;
}

In ngneat/spectator v16 we had:

export interface SpectatorOverrides<C> extends BaseSpectatorOverrides {
    detectChanges?: boolean;
    props?: Partial<C>;
}

and now in v17 we have:

export interface SpectatorOverrides<C> extends BaseSpectatorOverrides {
    detectChanges?: boolean;
    props?: InferInputSignals<C>;
}

As we can see earlier props was of type Partial<C> so we could pass values to component properties like age and id in this way:

spectator = createComponent({
    props: {
      age: 12,
      id: 13,
    },
  });

but now since props is of type InferInputSignals<C> we can no longer pass values like age and id but can only pass input() type variable values (eg. name). Is there any way to pass/assign values to props which are not input() type?

NetanelBasal commented 7 months ago

@kfrancois

profanis commented 7 months ago

In the previous version, we could also provide via the props the input despite its alias name.

@Component({ ... })
export class XyzComponent {
  name = input('userName');
}
spectator = createComponent({
    props: {
      name: 'John Doe'
    },
  });

In 17.1.0 this is not working and we have to provide the input using spectator.setInput('userName', 'John Doe'). Is it possible/good-idea to support these inputs via props?

kfrancois commented 7 months ago

Hi @ashutoshpaul!

I think the current behavior is valid - it's still possible to edit fields by doing something like:

const spectator = createComponent();

spectator.component.age = 12;
spectator.component.id = 13;

The reason I believe the current behavior is correct, is that only input fields (IE: fields marked with @Input or input-functions) should be set through inputs.

This simulates Angular's own behaviour, where you wouldn't be able to do something like this:

<app-xyz [age]="12" />

Angular throws an error for this code as age is not marked as an input, and I believe Spectator should behave the same way.

Now, while I strongly believe that we should only test a component's public API surface, I do see the possible utility in a way to assign values to non-input class fields. It depends mostly on how opinionated this library should be about best practices for Angular components. Something like fields: ExcludeInputSignals<C>; (added here) could work, even though I would discourage using it.

NetanelBasal commented 7 months ago

The reason I believe the current behavior is correct, is that only input fields (IE: fields marked with @Input or input-functions) should be set through inputs.

Agree with this statement

NetanelBasal commented 7 months ago

Maybe we should change props to inputs?

profanis commented 7 months ago

Maybe we should change props to inputs?

This sounds like a great idea.

NetanelBasal commented 7 months ago

It'll be better to deprecate the props field for now and introduce the inputs field