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

Upgrade to React 0.14.0 #3

Closed
wants to merge 2 commits into from
Closed

Conversation

jdlehman
Copy link

@jdlehman
Copy link
Author

This is a start to getting everything working properly with 0.14.0. I was able to get all the deprecation warnings out of the tests and onto the latest version of redux, but I did not get the tests passing yet.

Feel free to take a look and provide feedback on this progress and I will take another stab at getting the tests passing tomorrow.

@jdlehman
Copy link
Author

Went back and made the first commit simply upgrade React to 0.14.0 and things that did not necessitate changing the tests.

Everything passes but there are still deprecation warnings, so I will get rid of those and this will be ready to go.

@jdlehman
Copy link
Author

Take two on getting the tests passing without deprecations: All the deprecations are gone from the tests, but the batchedUpdate case is failing. I do not believe that the error is from the update to React or redux, but I must have done something wrong logically when refactoring.

This PR gets us most of the way there, but a second pair of eyes would be great to help me find the flaw in my test refactor logic.

}
}
const ConnectedComponent = connect(state => {
return { todos: state.todos, f: () => {} };
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the test failure has something to do with using the new connect function rather than the Connector component.

If the function f is included in the returned state, the case without batchedUpdates passes and the one with fails. Removing f from the returned state reverses this such that the batchedUpdates case passes and the case without fails.

@hung-phan
Copy link

👍

@sanemat
Copy link

sanemat commented Nov 3, 2015

ping @acdlite

@nickdima
Copy link

nickdima commented Dec 2, 2015

@jdlehman maybe you could publish your fork on npm meanwhile?

@jdlehman
Copy link
Author

jdlehman commented Dec 3, 2015

@nickdima you can point to my branch with npm if you'd like to use it: "redux-batched-updates": "git://github.com/jdlehman/redux-batched-updates.git#upgrade-react"

That said if you are concerned about performance, the update in redux 3.0.3 helps (though it doesn't replace the batched updates).

@rickharrison
Copy link

I'd recommend using https://github.com/tappleby/redux-batched-subscribe, which supports React 0.14

@nickdima
Copy link

nickdima commented Dec 3, 2015

great, thanks @rickharrison

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.

5 participants