reduxjs / redux

A JS library for predictable global state management
https://redux.js.org
MIT License
60.51k stars 15.26k forks source link

Could “ plain” class instances be stored in Redux store state. #4649

Closed michael-freidgeim-webjet closed 4 months ago

michael-freidgeim-webjet commented 4 months ago

Section https://redux.js.org/style-guide/#do-not-put-non-serializable-values-in-state-or-actions has a statement “Avoid putting non-serializable values such as Promises, Symbols, Maps/Sets, functions, or class instances into the Redux store state” “class instances” sounds more restrictive that it should be. I have a collection of typescript class instances, that has properties as primitives or other smaller typescript classes. The classes have data fields, but not behavior(such an events).
According to https://github.com/reduxjs/redux/issues/1407#issuecomment-184351915 “we recommend that both actions and the state are plain objects (or something like Immutable Maps)” ( referenced from https://redux.js.org/faq/organizing-state#can-i-put-functions-promises-or-other-non-serializable-items-in-my-store-state)

Could you please add a clarification in the rule, which class instances (or maps) are allowed and which are not recommended .

Currently it sounds, that I have to destruct all properties of my classes and store them as separate arrays, which defeat the purpose of having classes as not trivial data structures.

EskiMojo14 commented 4 months ago

i wouldn't recommend taking a comment from 2016 as gospel 😛 (we haven't recommended usage of ImmutableJS for a long time)

for the purposes of devtools and persistence, anything you can't do JSON.parse(JSON.stringify(value)) and get an equivalent value out of is not serializable, however "plain" it may seem. with this definition, no class instances are serializable - only POJOs, arrays and primitives.

michael-freidgeim-webjet commented 4 months ago

@EskiMojo14 “anything you can't do JSON.parse(JSON.stringify(value)) and get an equivalent value out of is not serializable” Thanks for the clear definition of “not serializable”, it’s worth to add as a link to clarify “ non-serializable” in the rule. Could you please provide more clarification, why Typescript class instances can’t be POJO? Or, in other terms, how to declare POJO objects in Typescript, if classes are not suatable?

EskiMojo14 commented 4 months ago

A POJO is something like { foo: "bar" }. As soon as you use the class keyword, it's no longer a POJO:

class Foo {
  foo = "bar"
}
console.log(JSON.parse(JSON.stringify(new Foo())) instanceof Foo) // false
michael-freidgeim-webjet commented 4 months ago

@EskiMojo14 I am still unclear, how to declare POJO objects in Typescript? Are classes with parameter properties suggested In https://stackoverflow.com/questions/41067961/what-to-use-for-data-only-objects-in-typescript-class-or-interface the only way?

EskiMojo14 commented 4 months ago

like i said, you make typically plain javascript object with the object literal syntax. no class keyword at all

phryneas commented 4 months ago
interface Person {
  firstname: string
  lastname: string
}

const myPersonThatIsNotAClass: Person = {
  firstname: "foo",
  lastname: "bar"
}

Really... just use objects. Classes are not a "TypeScript feature", classes are part of JavaScript since 2015, and TypeScript doesn't add anything on top. Just because you use TypeScript doesn't mean you suddently have to start using classes for everything. You have absolutely no need to use classes and especially if you have no additional methods on them, you have absolutely no benefits from using classes instead of plain objects.

(Also see the accepted answer to the question you linked to. It doesn't recommend classes.)

michael-freidgeim-webjet commented 4 months ago

Thanks, @phryneas . I will use interfaces instead of classes. It will be good to mention in the rule, that to define redux store strictures in Typescript should be used interfaces, because classes are not serialisable.

markerikson commented 4 months ago

The guidance is still accurate: you still should not put any class instance of any kind into Redux state.

Technically speaking, the code will run. But, classes are not meant for immutable updates, and they won't serialize properly.

michael-freidgeim-webjet commented 4 months ago

@markerikson , should the rule not only say what not to do, but suggest how to do it correctly? Classes are naively looks as a good choice to define structure in Typescript.

markerikson commented 4 months ago

@michael-freidgeim-webjet the docs are pretty clear that you should only put plain JS objects/arrays/primitives into state:

michael-freidgeim-webjet commented 4 months ago

@markerikson , sorry, but neither of 4 you provided links discuss “non-serializable” values.

I still believe that in the rule it is not obvious that the class instances are non-serializable and they can’t be considered as POJOs.(I am a full stack developer with more experience in C#, rather than JS)

I am fully understand that “Style Guide" is a concise summary of approaches, but it will be good to provide hyper-links that allow to clarify/explain statements of the rule. Other rules have “Detailed Explanation” section.

It’s just my feedback as a reader, it’s up to you to decide is it beneficial for other readers

markerikson commented 4 months ago

@michael-freidgeim-webjet I'm curious, what specific information would you like to see added / were you expecting to see? Something specifically saying "class instances aren't serializable", something saying why class instances aren't serializable, or something else?

michael-freidgeim-webjet commented 4 months ago

@markerikson I suggest something like the following:

Note that instead of class instances (that are non- serialisable) you can use POJO, in Typescript describe structure using interfaces/types

I’ve provided links, that I found today to understand the question, but you may find better references As it’s only 1 sentence, you probably do not need to hide it under “Detailed Explanation”

phryneas commented 4 months ago

Classes are naively looks as a good choice to define structure in Typescript.

I will repeat the point I made earlier, because I think you might be coming from another programming language and be missing a very central point about the programming language you are currently using:

Classes are not a TypeScript feature

Classes are a JavaScript feature. Nothing about classes is specific to TypeScript. TypeScript only adds type information in your editor, but it doesn't add features to your programming language - you are programming in JavaScript.

JavaScript has classes class Foo {}; new Foo, arrays [1,2,3] and objects { foo: "bar" }.

If you want to store data, use an object or an array.

If you want data with attached logic, use a class.

Storing data without attached logic in a class is a misuse of features of your programming language - this has nothing to do with Redux and everything with learning the programming language you are using.

michael-freidgeim-webjet commented 4 months ago

@markerikson you may also add a reference from section https://redux.js.org/style-guide/#do-not-put-non-serializable-values-in-state-or-actions to another rule below https://redux.js.org/style-guide/#use-plain-javascript-objects-for-state