Skip to content

refactor(types): change createStore types for a clearer error message#1157

Merged
ryansolid merged 1 commit intosolidjs:nextfrom
otonashixav:refactor-types-clearer-store-error
Aug 15, 2022
Merged

refactor(types): change createStore types for a clearer error message#1157
ryansolid merged 1 commit intosolidjs:nextfrom
otonashixav:refactor-types-clearer-store-error

Conversation

@otonashixav
Copy link
Contributor

Specifically, this change targets these differences:

  • Initializing a store with a generic type which is not restricted to object now shows Argument of type 'T' is not assignable to parameter of type 'object | undefined'. Type 'T' is not assignable to type 'object' instead of Argument of type '[T]' is not assignable to parameter of type '{} extends T ? [store?: T | undefined, options?: { name?: string | undefined; } | undefined] : [store: object & T, options?: { name?: string | undefined; } | undefined]'.

  • Initializing a store with a non-generic, non-object type now shows that the expected parameter type is object | undefined instead of never (or {} | undefined if trying to use null).

Specifically, this change targets these differences:

- Initializing a store with a generic type which is not restricted to `object` now shows `Argument of type 'T' is not assignable to parameter of type 'object | undefined'. Type 'T' is not assignable to type 'object'` instead of `Argument of type '[T]' is not assignable to parameter of type '{} extends T ? [store?: T | undefined, options?: { name?: string | undefined; } | undefined] : [store: object & T, options?: { name?: string | undefined; } | undefined]'`.

- Initializing a store with a non-generic, non-object type now shows that the expected parameter type is `object | undefined` instead of `never` (or `{} | undefiend` if trying to use `null`).
@otonashixav otonashixav marked this pull request as ready for review August 11, 2022 10:41
Copy link

@3imed-jaberi 3imed-jaberi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm

@ryansolid ryansolid merged commit dc151b5 into solidjs:next Aug 15, 2022
ryansolid added a commit that referenced this pull request Aug 26, 2022
* new batching, fix #1001, fix #1062

* fix #1019, resource state

* pluggable storage on resource

* Fix createMutable to handle reactivity in case element is undefined

* Added tests

* better fixes, ssr spread improvements

* Add `has` trap for accurate `in` with stores and mutables (#1109)

* Add `has` trap for accurate `in` with stores and mutables

Fixes #1107

* Fix ?. typo

* Remove optional chaining ?.

* fix(types): fix mergeProps type when spreading array as params (#1028)

This fixes the return type of mergeProps when an array is spread as params: `mergeProps(...propsArray)`, `mergeProps(...propsArray, props)`, and `mergeProps(props1, ...propsArray, props2)`. Previously these calls would ignore the type of the array being spread and all props after the spread e.g.:

```ts
const merged = mergeProps({ a: 1 }, ...[{ b: 2 }], { c: 3 }); // { a: 1 }
```

- Allow mergeProps to be called with all manner of spread params and return the correct type.
- As a consequence of the above, mergeProps' type allows calling it with no params.
- Brought back `Simplify`, since it doesn't interfere with generics and improves readability of the result type.
- Simplified and added comments for component.ts type tests.

Additionally:
- Improved types when generic props are present.

Issues:
- This doesn't correctly type spreading multiple arrays however, since typescript doesn't allow two "rest" params: `someFn(...[1], ...["2"], 3)` is inferred as `T = [...(string | number)[], number]` and not `T = [...number[], ...string[], number]`. In this case the union of types in the array is merged into a single type, which may not be entirely accurate.

* Fix `Dynamic` types (#1116)

* Fix `Dynamic` types

* Fix `component` type

* Fix `DynamicProps`, `ComponentProps`; Add `ValidComponent`

* Fix `Dynamic` test for type fix

* Fix `Dynamic` test to use `id` instead of `title` for DOM compatibility

* Fix `component` to be required property

* fix #1112 fix built in functions on proxy failing pass through

* v1.5.0-beta.0

* cleanups on the server

* update and rework build system

* tweak turbo commands

* remove clean command

* update ci

* decrease dependency on symlink

* update lockfile

* more type issues

* fix build

* SSR performance improvement

* fix bug with ssr resolve

* bump

* Add build instructions to CONTRIBUTING (#1136)

* Add pnpm build instructions to CONTRIBUTING

* Fix Turbo link

* fix: add explicit extensions and exports fields

* errors on server while not flushed, many bug fixes

* feat(types): createResource improvements (#1130)

* feat(types): createResource improvements

- Add typable `refetching`/`refetch` with default type `unknown` for backwards compatibility.
- Remove unneeded generics.
- Refactor `createResource` parameters to reduce casting.
- Remove unneeded instances of casting, non-null assertations and `any`.
- Fix `onHydrated` type, removing `refetching`.
- Add `InitializedResource` types for overloads which define `initialValue`.
- Changed `Error` to return `never` instead of `undefined`, since reading resources in such a state throws, so the values will never be used.
- Updated docs slightly.
- Fixed a test slightly (type of an unused parameter from `string` to `unknown`).

Potential Issues:
- Adding the initialized types might slightly break userland functions. However adding them is necessary to differentiate `{ initialValue: undefined }` from omitting `initialValue`, and for cases where `T` includes `undefined` to be typed correctly.
- `store` should not need to accept `undefined` if `initialValue` is provided, but in order to make passing a generic `createSignal`-typed function work without an error, `init` and the type of the signal returned must be the same.
- In various places `undefined` can still appear if a resource errors and refetches. As such `mutate` and `store` also need to handle `undefined` correctly. This might break typing for existing `mutate` calls which are typed without `undefined`.

* test(types): add tests for createResource types

* refactor(types): fix and clean up createResource types

- Add `NoInfer` for options so that only the fetcher is used to infer the type of the resource
- Remove some unneeded casting

* feat(types): export initialized resource types

Co-authored-by: Ryan Carniato <ryansolid@gmail.com>

* simplify batching, reduce createComputed, fix #1122 enable transitions.

* bump

* remove computed from SuspenseList

* Remove computed from createProvider

* Remove a console.log (#1146)

* fixes to suspenselist

* useInitialValue on resources

* fix #1138 dialog type, fix #1147 untrack ref, fix #1151 untrack cleanup

* keyed control flow updates

* bump

* improve createSelector perf

* draft 1.5 changelog

* fix falsy check

* faster asset rendering

* children.toArray

* refactor(types): change `createStore` types for a clearer error message (#1157)

Specifically, this change targets these differences:

- Initializing a store with a generic type which is not restricted to `object` now shows `Argument of type 'T' is not assignable to parameter of type 'object | undefined'. Type 'T' is not assignable to type 'object'` instead of `Argument of type '[T]' is not assignable to parameter of type '{} extends T ? [store?: T | undefined, options?: { name?: string | undefined; } | undefined] : [store: object & T, options?: { name?: string | undefined; } | undefined]'`.

- Initializing a store with a non-generic, non-object type now shows that the expected parameter type is `object | undefined` instead of `never` (or `{} | undefiend` if trying to use `null`).

* `resource.value` and small tweaks to resources

* bump

* experimenting with nodenext

* fix

* delete resource.value, defer to further deliberation

* small naming tweaks

* bump

* update packages

* bump

* docs: add quotes to snippets (#1153)

* better option naming for resource

* add missing deps

* Update Readme (#1137)

* add missing type exports

* bump

* small updates

* untrack JSON.stringify to avoid reactive side-effects on serialization. (#1177)

* untrack JSON.stringify to avoid reactive side-effects on serialization.

* untrack JSON.stringify to avoid reactive side-effects on serialization.

* keep immdiately evaluated module code, below its indirect declared let dependencies.

Co-authored-by: Ryan Carniato <ryansolid@gmail.com>

Co-authored-by: Paolo Ricciuti <ricciutipaolo@gmail.com>
Co-authored-by: Erik Demaine <edemaine@mit.edu>
Co-authored-by: Xavier Loh <42372774+otonashixav@users.noreply.github.com>
Co-authored-by: Alexis H. Munsayac <alexis.munsayac@gmail.com>
Co-authored-by: modderme123 <modderme123@gmail.com>
Co-authored-by: Kirill Mironov <k.mironov@tinkoff.ru>
Co-authored-by: Milo <modderme123@users.noreply.github.com>
Co-authored-by: Seanghay Yath <seanghay.dev@gmail.com>
Co-authored-by: Mathieu Decaffmeyer <5883963+mathieuprog@users.noreply.github.com>
Co-authored-by: LiQuidProQuo <105608035+LiQuidProQuo@users.noreply.github.com>
@otonashixav otonashixav deleted the refactor-types-clearer-store-error branch July 26, 2023 15:06
lilybw pushed a commit to lilybw/solid that referenced this pull request Nov 12, 2025
* new batching, fix solidjs#1001, fix solidjs#1062

* fix solidjs#1019, resource state

* pluggable storage on resource

* Fix createMutable to handle reactivity in case element is undefined

* Added tests

* better fixes, ssr spread improvements

* Add `has` trap for accurate `in` with stores and mutables (solidjs#1109)

* Add `has` trap for accurate `in` with stores and mutables

Fixes solidjs#1107

* Fix ?. typo

* Remove optional chaining ?.

* fix(types): fix mergeProps type when spreading array as params (solidjs#1028)

This fixes the return type of mergeProps when an array is spread as params: `mergeProps(...propsArray)`, `mergeProps(...propsArray, props)`, and `mergeProps(props1, ...propsArray, props2)`. Previously these calls would ignore the type of the array being spread and all props after the spread e.g.:

```ts
const merged = mergeProps({ a: 1 }, ...[{ b: 2 }], { c: 3 }); // { a: 1 }
```

- Allow mergeProps to be called with all manner of spread params and return the correct type.
- As a consequence of the above, mergeProps' type allows calling it with no params.
- Brought back `Simplify`, since it doesn't interfere with generics and improves readability of the result type.
- Simplified and added comments for component.ts type tests.

Additionally:
- Improved types when generic props are present.

Issues:
- This doesn't correctly type spreading multiple arrays however, since typescript doesn't allow two "rest" params: `someFn(...[1], ...["2"], 3)` is inferred as `T = [...(string | number)[], number]` and not `T = [...number[], ...string[], number]`. In this case the union of types in the array is merged into a single type, which may not be entirely accurate.

* Fix `Dynamic` types (solidjs#1116)

* Fix `Dynamic` types

* Fix `component` type

* Fix `DynamicProps`, `ComponentProps`; Add `ValidComponent`

* Fix `Dynamic` test for type fix

* Fix `Dynamic` test to use `id` instead of `title` for DOM compatibility

* Fix `component` to be required property

* fix solidjs#1112 fix built in functions on proxy failing pass through

* v1.5.0-beta.0

* cleanups on the server

* update and rework build system

* tweak turbo commands

* remove clean command

* update ci

* decrease dependency on symlink

* update lockfile

* more type issues

* fix build

* SSR performance improvement

* fix bug with ssr resolve

* bump

* Add build instructions to CONTRIBUTING (solidjs#1136)

* Add pnpm build instructions to CONTRIBUTING

* Fix Turbo link

* fix: add explicit extensions and exports fields

* errors on server while not flushed, many bug fixes

* feat(types): createResource improvements (solidjs#1130)

* feat(types): createResource improvements

- Add typable `refetching`/`refetch` with default type `unknown` for backwards compatibility.
- Remove unneeded generics.
- Refactor `createResource` parameters to reduce casting.
- Remove unneeded instances of casting, non-null assertations and `any`.
- Fix `onHydrated` type, removing `refetching`.
- Add `InitializedResource` types for overloads which define `initialValue`.
- Changed `Error` to return `never` instead of `undefined`, since reading resources in such a state throws, so the values will never be used.
- Updated docs slightly.
- Fixed a test slightly (type of an unused parameter from `string` to `unknown`).

Potential Issues:
- Adding the initialized types might slightly break userland functions. However adding them is necessary to differentiate `{ initialValue: undefined }` from omitting `initialValue`, and for cases where `T` includes `undefined` to be typed correctly.
- `store` should not need to accept `undefined` if `initialValue` is provided, but in order to make passing a generic `createSignal`-typed function work without an error, `init` and the type of the signal returned must be the same.
- In various places `undefined` can still appear if a resource errors and refetches. As such `mutate` and `store` also need to handle `undefined` correctly. This might break typing for existing `mutate` calls which are typed without `undefined`.

* test(types): add tests for createResource types

* refactor(types): fix and clean up createResource types

- Add `NoInfer` for options so that only the fetcher is used to infer the type of the resource
- Remove some unneeded casting

* feat(types): export initialized resource types

Co-authored-by: Ryan Carniato <ryansolid@gmail.com>

* simplify batching, reduce createComputed, fix solidjs#1122 enable transitions.

* bump

* remove computed from SuspenseList

* Remove computed from createProvider

* Remove a console.log (solidjs#1146)

* fixes to suspenselist

* useInitialValue on resources

* fix solidjs#1138 dialog type, fix solidjs#1147 untrack ref, fix solidjs#1151 untrack cleanup

* keyed control flow updates

* bump

* improve createSelector perf

* draft 1.5 changelog

* fix falsy check

* faster asset rendering

* children.toArray

* refactor(types): change `createStore` types for a clearer error message (solidjs#1157)

Specifically, this change targets these differences:

- Initializing a store with a generic type which is not restricted to `object` now shows `Argument of type 'T' is not assignable to parameter of type 'object | undefined'. Type 'T' is not assignable to type 'object'` instead of `Argument of type '[T]' is not assignable to parameter of type '{} extends T ? [store?: T | undefined, options?: { name?: string | undefined; } | undefined] : [store: object & T, options?: { name?: string | undefined; } | undefined]'`.

- Initializing a store with a non-generic, non-object type now shows that the expected parameter type is `object | undefined` instead of `never` (or `{} | undefiend` if trying to use `null`).

* `resource.value` and small tweaks to resources

* bump

* experimenting with nodenext

* fix

* delete resource.value, defer to further deliberation

* small naming tweaks

* bump

* update packages

* bump

* docs: add quotes to snippets (solidjs#1153)

* better option naming for resource

* add missing deps

* Update Readme (solidjs#1137)

* add missing type exports

* bump

* small updates

* untrack JSON.stringify to avoid reactive side-effects on serialization. (solidjs#1177)

* untrack JSON.stringify to avoid reactive side-effects on serialization.

* untrack JSON.stringify to avoid reactive side-effects on serialization.

* keep immdiately evaluated module code, below its indirect declared let dependencies.

Co-authored-by: Ryan Carniato <ryansolid@gmail.com>

Co-authored-by: Paolo Ricciuti <ricciutipaolo@gmail.com>
Co-authored-by: Erik Demaine <edemaine@mit.edu>
Co-authored-by: Xavier Loh <42372774+otonashixav@users.noreply.github.com>
Co-authored-by: Alexis H. Munsayac <alexis.munsayac@gmail.com>
Co-authored-by: modderme123 <modderme123@gmail.com>
Co-authored-by: Kirill Mironov <k.mironov@tinkoff.ru>
Co-authored-by: Milo <modderme123@users.noreply.github.com>
Co-authored-by: Seanghay Yath <seanghay.dev@gmail.com>
Co-authored-by: Mathieu Decaffmeyer <5883963+mathieuprog@users.noreply.github.com>
Co-authored-by: LiQuidProQuo <105608035+LiQuidProQuo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants