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

Use removedev-serialize instead of transit #25

Merged
merged 1 commit into from
Oct 10, 2017
Merged

Conversation

ntucker
Copy link
Contributor

@ntucker ntucker commented Oct 8, 2017

This will reduce bundle size, and theoretically make it compatible with immutable 4 as well.

Tested by yarn-linking this part to my main project, and everything worked.

This was referenced Oct 8, 2017
@rt2zz
Copy link
Owner

rt2zz commented Oct 10, 2017

awesome thank you!

@rt2zz rt2zz merged commit 855ce86 into rt2zz:master Oct 10, 2017
@ntucker
Copy link
Contributor Author

ntucker commented Oct 10, 2017

FYI: This will result in previous persisted things not loading right since they are serialized in a different way. There should at least be some warnings about the change (an major version bump) so people can handle those cases.

@rt2zz
Copy link
Owner

rt2zz commented Oct 10, 2017

right, I was going to wait some time to cut a release, also to get an answer here zalmoxisus/remotedev-serialize#4

You bring up a good point, the migration will be painful, probably best handled with a major version bump + a minor with deprecation warnings. That or we could consider moving this into redux-persist core.

@tborg
Copy link

tborg commented Dec 22, 2017

Say @rt2zz, I wonder if you've thought about cutting a new release for this lately? At this point it seems like the crickets at zalmoxisus/remotedev-serialize#4 indicates a lack of objections.

I have pointed my package.json at master and it seems to work great. If you're strapped for time I could have a go at implementing a minor version with deprecation warnings.

@chrisgbaker
Copy link

Has there been any more thoughts on this? Definitely interested in moving away from having transit-js in our bundles.

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.

4 participants