carbon-design-system / carbon-components-svelte

Svelte implementation of the Carbon Design System
https://svelte.carbondesignsystem.com
Apache License 2.0
2.68k stars 261 forks source link

fix(text-area): type `value` prop as nullable #1933

Closed CordlessWool closed 1 month ago

CordlessWool commented 6 months ago

Allow null as value attribute on TextArea as on TextInput

brunnerh commented 6 months ago

I think that it makes sense to loosen the value type to accept null so that it works in TypeScript strict mode.

If the Svelte language tools were actually strict, this would just create problems elsewhere. With null in the type, it should not be possible to bind:value to a string variable any more because null is not assignable to string.

If this were to cause errors on all existing bindings, I would be quite opposed to this, but it looks like this is not the case...

CordlessWool commented 6 months ago

Since when is null related to number, null is not 0. To be 100% sure I also made a short implementation and I didn't had any problems with combining null, undefinded and string in combination with a bind. TS is in strict mode.

brunnerh commented 6 months ago

Since when is null related to number, null is not 0.

That was exactly my point, null is not compatible with other types, which is why such assignments are not allowed by TS itself. The Svelte language tools (conveniently?) ignore this.

Pure TS example:

let componentProperty: string | null = {} as any;
let variableThatIsBound: string = {} as any;

variableThatIsBound = componentProperty;

Error on variableThatIsBound:

Type 'string | null' is not assignable to type 'string'.
  Type 'null' is not assignable to type 'string'.(2322)
CordlessWool commented 6 months ago

Yes and exacatly this is why we need null as option because we have a variable that could be null | undefined | string and this is not assign able to undefined | string. To handle undefiend or null will make no difference for the component.

Undefiend will also not assign able to string and anyway it is allowed, so the component already handle it and you will be able to assign string to string | null.

metonym commented 1 month ago

Fixed in v0.85.1