search

quarrant / mobx-persist-store

Persist and rehydrate observable properties in mobx store.
268 stars 14 forks source link

Added serializable properties #88

Closed quarrant closed 2 years ago

quarrant commented 2 years ago

@codeBelt Look at this PR and tell me your opinion about serialize/deserialize. Tests are broken here, it's OK! =)

quarrant commented 2 years ago

FYI: I tried to pass right types through the each related interface, but I'm not sure about return types of serialize/deserialize functions. May be I need to make those more strict....

quarrant commented 2 years ago

@codeBelt do you approve this PR?

codeBelt commented 2 years ago

@quarrant sure approved. Does this need to be a major release 2.0.0 or is it 1.1.0?

quarrant commented 2 years ago

So, I think this feature is awesome, but still small for the 2.0.0=)

codeBelt commented 2 years ago

We would need to make it 2.0 if this update breaks existing functionality for people. If this change works like the previous version then 1.1.0 is what I think we should make it since we are at 1.0.6

quarrant commented 2 years ago

Yeah, I understand) But these changes support old approach too

quarrant commented 2 years ago

Anyway, not important for me. We will increase major version if you want =)

codeBelt commented 2 years ago

I think we should go with 1.1.0 if there are not breaking changes.

quarrant commented 2 years ago

If you are not sure please see again into PR. I added only 1 test for the new functionality and didn't change other tests and it works good.

makeSerializableProperties (https://github.com/quarrant/mobx-persist-store/pull/88/files#diff-9b6ae0b42cdb023ac583e5e708e01559e766f2388fc8c808c7ca75048bf3ac05R29) transforms string properties (old way) to SerializableProperty (new way)

codeBelt commented 2 years ago

sorry, really busy with work but it all looks good.

quarrant commented 2 years ago

no worries about it, I'm toooooo... Thank you for your comments!