solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.05k stars 914 forks source link

`null` and `undefined` are missing in `createSignal` return type #972

Closed julvo closed 2 years ago

julvo commented 2 years ago

Describe the bug

When creating a signal with a union type that includes null or undefined, the resulting signal doesn't include null or undefined.

E.g. if we create a signal that can be null or a number using createSignal<null | number>(null), the return type is Signal<number> instead of Signal<number | null>

The issue is that the value of the signal is actually null, but the type signature doesn't reflect that. That means we loose type-safety.

Your Example Website or App

https://playground.solidjs.com/?version=1.3.13#NobwRAdghgtgpmAXGGUCWEwBowBcCeADgsrgM4Ae2YZA9gK4BOAxiWGjIbY7gAQi9GcCABM4jXgF9eAM0a0YvADo1aAGzQiAtACsyAegDucAEYqA3EogcuPfr2ZCouOAGU0Ac2hqps+YpU6DW09CysrGXoIZlw0WgheAGEGCBdGAAoASn4rXgd4sj5gZhTcLF4yOFxkqNwAXV4AXgcnF3cvKDUAHgh6GBNxXgAfXl61NQA+dLG1TMsE-IhC3gxHOHhUpt4spomKqprU9JLanYBqXgBGOdzlBf19XgADKCeVsl5aGV4CYmfe-riN4meh8MgACwYahEvAG-z6AwkIxmT1uJSWfCgWxOR0y4QWQlwTAS6VueS6INwuHiPyIcEaKkp1IgKk+EESGmYAGtGiBVkINrhJBMyXl+DjcFlJKK8iAoNKFuT9Ez4iKFjcIAqrEJROJ0jtGnsuoc0rx9BNyiJaMw+sJcAA6DxVACianWdoAQvgAJIiUlgKCEQgqTIasCSOpAA

Steps to Reproduce the Bug or Issue

  1. Create a signal with a nullable type, e.g. number | null
  2. Observe that resulting signal type doesn't include null, e.g. only number instead of number | null

Expected behavior

When using createSignal<T | null>(null), I'd expect the resulting signal to be of type Signal<T | null>

Screenshots or Videos

Example from the Solid playground with createSignal<number | null>()

image (18)

Also an issue when creating a nullable signal using createSignal<number>(), where I'd expect the return type to be Signal<number | undefined>

image (19)

Platform

Additional context

The issue may be related to #217, but that issue was focused on initialising without arguments.

edemaine commented 2 years ago

This is just a bug with the Playground: https://github.com/solidjs/solid-playground/issues/41

In actual use with TypeScript's strict: true turned on, null/undefined are properly preserved:

image

ryansolid commented 2 years ago

Yeah I'm going to close this one as a duplicate. We have some plans with the playground that will help with this.

martinpengellyphillips commented 2 years ago

Hmmm, I see a related issue here still with the setter typing. The setter is only accepting the first value from the union type. Let me know if I should open an issue for this (or perhaps I am just doing something wrong here - I'm a typescript novice still)

This is in vscode:

createsignal-setter-type-issue

edemaine commented 2 years ago

This is still actually working. Notice the <string> in <string>(value: string | ...) => string. This means that the type of setReferenceDraggableId is parameterized, and here it's being used in its string form, presumably because draggable.id is of type string.

The type definition is complicated, to prevent accidental attempts of passing in a function. But it should do what you actually want (albeit making hard-to-read types).

martinpengellyphillips commented 2 years ago

draggable.id is string | number though 🤔

And I see this warning (the squiggly lines in the previous image):

(property) id: string | number
Argument of type 'string | number' is not assignable to parameter of type 'string | ((prev: ID) => string)'.
  Type 'number' is not assignable to type 'string | ((prev: ID) => string)'.ts(2345)
edemaine commented 2 years ago

It looks like #978 will fix this. Thanks @otonashixav!