-
-
Notifications
You must be signed in to change notification settings - Fork 177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Angular 17 - input with transform #653
Comments
@kfrancois, I guess this could be connected to #637. |
@LukasMachetanz thank you for logging this issue! Before we can work on this issue, could you provide some more information around the expected/current behaviour, or perhaps a reproduction? I've tried the example you mentioned, and I'm not sure what should change around the current behaviour. Note that this commit addressed input signals with In your example, At the moment, this is a conscious choice (see discussion here #649). If you believe this is wrong, we can re-open the discussion around this. I hope this helps already! |
@kfrancois, thank you for the fast reply. Let's assume having the following component:
And let's assume having the following test:
I would assume that it is possible to provide Therefore, I am wondering if I am understanding something wrong or if the behaviour within the test is really by intention? Any help appreciated. :) |
@kfrancois, do you have any opinion on this? |
@LukasMachetanz sorry for my slow response here - my personal opinion is that the current behaviour makes sense in most cases, as we'd want to pass in the type that the component actually uses (rather than the type that the transform function will accept). My reasoning here is:
However, I understand that my opinion might not be universally applicable. As such, we could make the typing of InferInputSignal weaker to allow your proposal (basically by flipping If we were to make this change, the drawback is that in the following example: public id = input.required({ transform: numberAttribute }); The inferred type for |
@kfrancois, thank you for responding and sharing your opinion. I fear that I see this differently though. I would expect to test against the public interface of Regarding the inferred type: In my opinion the inferred type should be the accepted type of the transform function. In the case of |
@kfrancois, do you have a final take on this? :) |
Sorry @LukasMachetanz, I thought I responded to this already - I can agree with your reasoning, would you like to drop a PR which swaps the generic type for this? I believe the change should just consist of this: export type InferInputSignal<T> = T extends InputSignalWithTransform<infer _, infer K> ? K : T; Thank you, and sorry again for taking so long to get back to you! |
I also have another issue that I'm not sure if others also have it. The code that I have works, but I get that annoying error. |
Is this a regression?
No
Description
Spectator does not correctly work using
input
havingtransform
configured.public id = input.required<number, string>({ transform: numberAttribute });
I would expect that Spectator correctly deals with this signature as well.
Please provide a link to a minimal reproduction of the bug
No response
Please provide the exception or error you saw
No response
Please provide the environment you discovered this bug in
No response
Anything else?
No response
Do you want to create a pull request?
No
The text was updated successfully, but these errors were encountered: