Deprecations before v4 #715

devanshj · 2021-12-17T00:03:50Z

This PR is to be released as a minor which will contain deprecations that'll make the migration to the next major smoother. This is not to be merged yet, as #662 progresses this will get more commits if required, so this will be ready when #662 is ready.

I'm quite an idealist, I value correctness higher than non-breaking change, so changes are in alignment to that, they can always be negotiated later, I'm not the boss :)

Changes till now:

Minimizing Types.
- Some types are removed which can be replaced by a existing type. Eg SetState<T> to Store<T>['setState']. It's best if derived types are actually derived. A type named SetState<T> doesn't tell what is it's relationship to Store<T> or where did it came from. It's always good to only export the necessary types.
- Some types are removed which can be inlined. Eg EqualityChecker<T> to (a: T, b: T) => boolean and StateSelector<T, U> to (state: T) => U. The beauty of types is that you can tell their implementation by simply looking at them. StateSelector<T, U> doesn't tell me anything about the implementation whereas a shorter (state: T) => U does. Types should only be extracted if the name is more telling than it's definition or the definition is too long to write, eg Store<T>.
- Some types are only renamed to better names. Eg. StateCreator to StoreInitializer.
Removing the "api" nomenclature.
Calling "store" as "storeApi" is like calling "todos" as "todosArray", "Tea" as "Tea beverage", "Devansh" as "Devansh Human". Good part is that this is not visible at a lot of places in the code. When using context, there's a useStoreApi so that is renamed to useStoreRef (as useStore is already for reading the state).
A more idealist and correct naming would have been calling MyStoreContext.useStore() as simply MyStoreContext.use(), and MyStoreContext.useStoreRef() as MyStoreContext.useStore(), but that's too radical change so I avoided it, let me know if you like it anyway.
devtools({}, name) to devtools({}, { name }. Ability to pass the name directly is a rather unnecessary feature, more over expecting an object is consistent with persist. Good part is the documentation doesn't mention that you can directly pass the name so hopefully many users would not be using it.

codesandbox-ci · 2021-12-17T00:04:08Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7e3582b:

Sandbox	Source
React	Configuration
React Typescript	Configuration
React Browserify	Configuration
React Snowpack	Configuration
React Parcel	Configuration
Next.js	Configuration

devanshj · 2021-12-21T22:59:59Z

@dai-shi #662 is almost ready, should we go ahead with these changes? Feel free to disagree.

dai-shi · 2021-12-22T12:18:47Z

will check this later. can you fix ci errors, meanwhile?

dai-shi · 2021-12-23T08:35:28Z

Okay, I looked at the description (not the code yet), but this can't be accepted.
You want to assume that if there are too many breaking changes, people would lose trust and leave for another library.

Here are some overall comments:

I personally like minimizing types. But, this can be tough for TS beginners, and we will see many complaints. Let's drop some types if they are impossible (or maybe not reasonable) to implement in Rewrite with better types #662 and we can give a migration path easily with @deprecated message.
Same for renaming types. We should keep them as much as possible, unless we have more benefit with renaming, or unless old ones are too confusing.
Please do not change the JS code in zustand/context. We will keep the same functionality in v4 as much as possible. (tbh, I want to drop zustand/context entirely, but dropping will be troublesome too.) I understand the naming isn't perfect, but the history (= keeping backward compatibility) is more important.
devtools(..., { name }) seems natural and bundle size increase is usually not a big issue for DEV only things. I'm not sure how many uses directly pass name. If there are many we might consider keeping the deprecation message in v4 (I think unlikely, though).

I'm still open for suggestions about dropping and renaming types, but generally we need to consider more about backward compatibility.

dai-shi · 2021-12-23T08:37:56Z

The devtools signature change/deprecation can be in v3.6.8. What do you think?

devanshj · 2021-12-23T21:36:16Z

I personally like minimizing types. But, this can be tough for TS beginners, and we will see many complaints. Let's drop some types if they are impossible (or maybe not reasonable) to implement in Rewrite with better types #662 and we can give a migration path easily with @deprecated message.
Same for renaming types. We should keep them as much as possible, unless we have more benefit with renaming, or unless old ones are too confusing.

I think we should make users, including TS beginners, adapt to the types instead of spoon-feeding them with types as trivial as EqualityChecker<T>. My suggestions for the type changes are in the PR, let me know which changes you like and which you don't. If you want to keep types as backward compatible as possbile, that can be done too, as I've done in #725

Please do not change the JS code in zustand/context. We will keep the same functionality in v4 as much as possible. (tbh, I want to drop zustand/context entirely, but dropping will be troublesome too.) I understand the naming isn't perfect, but the history (= keeping backward compatibility) is more important.

Okay haha I understand, we'll keep it backward compatible.

devtools(..., { name }) seems natural and bundle size increase is usually not a big issue for DEV only things. I'm not sure how many uses directly pass name. If there are many we might consider keeping the deprecation message in v4 (I think unlikely, though).

The devtools signature change/deprecation can be in v3.6.8. What do you think?

Yep that sounds good

dai-shi · 2021-12-23T23:40:57Z

Added a comment in #725.

My suggestions for the type changes are in the PR, let me know which changes you like and which you don't.

I'll review the suggestions again with the new approach in mind.

The devtools signature change/deprecation can be in v3.6.8. What do you think?

Yep that sounds good

Please go ahead and create a PR, then v3.6.8 will be ready.

devanshj · 2021-12-24T01:46:57Z

Done #731

dai-shi

Reviewed vanilla.ts. It turns out, I'd be more conservative.

dai-shi · 2021-12-25T05:10:13Z

src/vanilla.ts

@@ -1,6 +1,18 @@
+/**
+ * @deprecated `State` is renamed to `UnknownState`,
+ * `State` will be removed in next major


I would wanted to drop State entirely, and not even export UnknownState.
But, this will probably be too big change, so let's keep it.

I would even want to make it unknown. If it were possible, we could consider dropping.

dai-shi · 2021-12-25T05:13:23Z

src/vanilla.ts

+/**
+ * @deprecated Use the builtin `Partial<T>` instead of `PartialState<T>`.
+ * Additionally turn on `--exactOptionalPropertyTypes` tsc flag.
+ * `PartialState` will be removed in next major


Yes, this has been troublesome and happy to drop it.

dai-shi · 2021-12-25T05:16:28Z

src/vanilla.ts

+/**
+ * @deprecated Use `(state: T, prevState: T) => void` instead of `StateListener<T>`.
+ * `StateListener` will be removed in next major.
+ */
 export type StateListener<T> = (state: T, previousState: T) => void


For StateSelector, EqualityChecker and StateListener, I'm not sure what will happen. I guess many users use them. They might not be necessary nor best named, but not too confusing. So, for now I lean to keep them.

I can try there three actually, and see if someone gets confused.

I can try there three actually, and see if someone gets confused.

Did you thought about this and decided that if you want to keep them or not?

Let's keep them.
I saw some tweets and codesandboxes and know people use them.
If we were to remove those, it would be v5 (taking migration period longer like a few months to a year), which will not happen any time soon.

dai-shi · 2021-12-25T05:22:35Z

src/vanilla.ts

+/**
+ * @deprecated Use `(slice: T, prevSlice: T) => void` instead of `StateSliceListener<T>`.
+ * `StateSliceListener` will be removed in next major.
+ */
 export type StateSliceListener<T> = (slice: T, previousSlice: T) => void


I think my plan was to drop this in #604. I don't know why I didn't. Let's drop this at least from vanilla.ts.

dai-shi · 2021-12-25T05:24:12Z

src/vanilla.ts

 export type StateSliceListener<T> = (slice: T, previousSlice: T) => void
+
+/**
+ * @deprecated Use `Store<T>['subscribe']` instead of `Subscribe<T>`.


I don't dislike this, but people would get confused. Better to stay. Same for SetState and GetState.

dai-shi · 2021-12-25T05:24:48Z

src/vanilla.ts

+/**
+ * @deprecated Use `() => void` or `Store<UnknownState>['destroy']` instead of `Destroy`.
+ * `Destroy` will be removed in next major.
+ */
 export type Destroy = () => void


I'm fine to drop this. Not many people should be using.

dai-shi · 2021-12-25T05:26:03Z

src/vanilla.ts

 export type Destroy = () => void
+
+/**
+ * @deprecated `StoreApi<T>` has been renamed to `Store<T>`


I know you don't like this, but let's keep it.

dai-shi · 2021-12-25T05:27:08Z

src/vanilla.ts

+}
+
+/**
+ * @deprecated `StateCreator` has been renamed to `StoreInitializer`.


Let's keep this.

devanshj · 2021-12-25T05:41:09Z

I would wanted to drop State entirely, and not even export UnknownState.
But, this will probably be too big change, so let's keep it.

For StateSelector, EqualityChecker and StateListener, I'm not sure what will happen. I guess many users use them. They might not be necessary nor best named, but not too confusing. So, for now I lean to keep them.

I don't dislike this, but people would get confused. Better to stay. Same for SetState and GetState.

These types are rather simple to replace, so I don't think it would be troublesome to migrate, but I'm fine if you want to keep them.

dai-shi · 2021-12-25T05:59:04Z

No, it's not about how troublesome they are to migrate. If people need to change something, people would hesitate. If they feel unhappy, they will leave. We are already planning many changes, so from your perspective, please think it as we should keep backward compatibility as much as possible. v3.6.x isn't nice for middleware typing. in this sense, middlware typing should be backward compatible with v3.5.x again as much as possible.

devanshj · 2022-02-02T05:53:50Z

Ha weird I thought the new branch renaming feature worked across forks xD — will reopen

devanshj added 3 commits December 17, 2021 05:32

devtools: deprecate passing name directly instead of an object

28b66c6

context: rename useStoreApi to useStoreRef

b42b37f

type deprecations

b5be4ac

devanshj marked this pull request as draft December 17, 2021 00:04

console.warn when using useStoreApi

1de740e

fix lint

7e3582b

dai-shi added this to the v3.7.0 milestone Dec 23, 2021

devanshj mentioned this pull request Dec 23, 2021

Rewrite with better types #662

Closed

10 tasks

devanshj mentioned this pull request Dec 24, 2021

breaking(types): Add higher kinded mutator types #725

Merged

devanshj mentioned this pull request Dec 24, 2021

fix(middleware/devtools): deprecate passing name directly instead of an object #731

Merged

dai-shi reviewed Dec 25, 2021

View reviewed changes

devanshj closed this Feb 2, 2022

devanshj deleted the devansh-deprecations branch February 2, 2022 05:51

devanshj mentioned this pull request Feb 2, 2022

feat: Migrations regarding higher kinded mutators for v4 #772

Merged

dai-shi removed this from the v3.7.0 milestone Feb 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecations before v4 #715

Deprecations before v4 #715

devanshj commented Dec 17, 2021 •

edited

Loading

codesandbox-ci bot commented Dec 17, 2021 •

edited

Loading

devanshj commented Dec 21, 2021

dai-shi commented Dec 22, 2021

dai-shi commented Dec 23, 2021

dai-shi commented Dec 23, 2021

devanshj commented Dec 23, 2021

dai-shi commented Dec 23, 2021

devanshj commented Dec 24, 2021

dai-shi left a comment

dai-shi Dec 25, 2021

dai-shi Dec 25, 2021

dai-shi Dec 25, 2021

dai-shi Dec 25, 2021

devanshj Jan 6, 2022

dai-shi Jan 6, 2022

dai-shi Dec 25, 2021

dai-shi Dec 25, 2021

dai-shi Dec 25, 2021

dai-shi Dec 25, 2021

dai-shi Dec 25, 2021

devanshj commented Dec 25, 2021 •

edited

Loading

dai-shi commented Dec 25, 2021

devanshj commented Feb 2, 2022

Deprecations before v4 #715

Deprecations before v4 #715

Conversation

devanshj commented Dec 17, 2021 • edited Loading

codesandbox-ci bot commented Dec 17, 2021 • edited Loading

devanshj commented Dec 21, 2021

dai-shi commented Dec 22, 2021

dai-shi commented Dec 23, 2021

dai-shi commented Dec 23, 2021

devanshj commented Dec 23, 2021

dai-shi commented Dec 23, 2021

devanshj commented Dec 24, 2021

dai-shi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devanshj commented Dec 25, 2021 • edited Loading

dai-shi commented Dec 25, 2021

devanshj commented Feb 2, 2022

devanshj commented Dec 17, 2021 •

edited

Loading

codesandbox-ci bot commented Dec 17, 2021 •

edited

Loading

devanshj commented Dec 25, 2021 •

edited

Loading