Skip to content

signal inputs seem to not work with numberAttribute (generic is needed, or needs inference) #53969

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

Open
cipchk opened this issue Jan 18, 2024 · 5 comments

Comments

@cipchk
Copy link

cipchk commented Jan 18, 2024

Which @angular/* package(s) are the source of the bug?

core

Is this a regression?

Yes

Description

@Directive({
  selector: 'auto-focus',
  standalone: true,
})
export class AutoFocus {
  delay = input<number>(300, { transform: numberAttribute });
}

Throw error:

Error in src/main.ts (10:32)
Type '(value: unknown, fallbackValue?: number | undefined) => number' is not assignable to type 'undefined'.

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/stackblitz-starters-fv1yrc?file=src%2Fmain.ts

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in (run ng version)

Angular CLI: 17.1.0
Node: 18.16.0
Package Manager: yarn 4.0.2
OS: darwin x64

Angular: 17.1.0
... animations, cdk, cli, common, compiler, compiler-cli, core
... elements, forms, language-service, platform-browser
... platform-browser-dynamic, platform-server, router
... service-worker, ssr

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1701.0
@angular-devkit/build-angular   17.1.0
@angular-devkit/core            17.1.0
@angular-devkit/schematics      17.1.0
@schematics/angular             17.1.0
ng-packagr                      17.1.0
rxjs                            7.8.1
typescript                      5.3.3
zone.js                         0.14.3

Anything else?

No response

@ngbot ngbot bot modified the milestone: needsTriage Jan 18, 2024
@JoostK
Copy link
Member

JoostK commented Jan 18, 2024

This is a bit of a gotcha, it currently needs to be

input<number, unknown>(300, { transform: numberAttribute });

The reason is that a transform introduces a divergence in the signal's type: it accepts unknown value whereas it contains number value.

I am not sure if this can be improved upon, as TypeScript currently requires all generic types to be set instead of inferring some of them (microsoft/TypeScript#54047)

@JoostK
Copy link
Member

JoostK commented Jan 18, 2024

Also, in this particular case, you can opt not to specify any type arguments at all. In that case everything is fully inferred. It's only the partial inference that doesn't work.

Still, the WriteT being the first type parameter may be undesirable if partial type inference ever becomes a thing. <- This was inaccurate, thanks Paul!

@devversion
Copy link
Member

devversion commented Jan 18, 2024

Thanks Joost for capturing all of this. We did consider this aspect and talked about it a couple of times. We don't see a good way around this, without other more impactful trade-offs. One correction, the type needs to be:

delay = input<number, unknown>(300, { transform: numberAttribute });

The first type parameter is actually the ReadT (what you as the author of the directive are dealing with), while the second parameter is the WriteT (i.e. what values can be bound to the input, and the transform converts to ReadT).

We are good, if TS supports partial type inference, the following will work as well. Right now, either need to be explicit, or fully rely on inference (which is safe here):

  // will works when TS supports partial type inference
  delay = input<number>(300, { transform: numberAttribute }); 

  // works right now
  delay = input<number, unknown>(300, { transform: numberAttribute });
  delay = input(300, { transform: numberAttribute });

@devversion devversion changed the title signal inputs can't use numberAttribute signal inputs seem to not work with numberAttribute (generic is needed, or needs inference) Jan 19, 2024
@pkozlowski-opensource
Copy link
Member

I'm going to mark this issue as blocked since the final solution should come from microsoft/TypeScript#54047

Also developers can relatively easily deal with this situation as pointed out by #53969 (comment)

@alixroyere
Copy link

Same issue with booleanAttribute (for search reference)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants
@JoostK @pkozlowski-opensource @cipchk @devversion @alixroyere and others