Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
michael-freidgeim-webjet opened this issue Dec 29, 2023 · 15 comments

Comments

@michael-freidgeim-webjet
Copy link

michael-freidgeim-webjet commented Dec 29, 2023

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
#1407 (comment)
“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
Copy link
Contributor

EskiMojo14 commented Dec 29, 2023

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
Copy link
Author

@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
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Contributor

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

@phryneas
Copy link
Member

phryneas commented Dec 30, 2023

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
Copy link
Author

michael-freidgeim-webjet commented Dec 30, 2023

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
Copy link
Contributor

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.

@markerikson markerikson closed this as not planned Won't fix, can't repro, duplicate, stale Dec 30, 2023
@michael-freidgeim-webjet
Copy link
Author

@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
Copy link
Contributor

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

@michael-freidgeim-webjet
Copy link
Author

michael-freidgeim-webjet commented Dec 30, 2023

@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
Copy link
Contributor

@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
Copy link
Author

@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
Copy link
Member

phryneas commented Dec 30, 2023

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
Copy link
Author

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

No branches or pull requests

4 participants