Add useReducer API #34
Conversation
| @@ -382,7 +379,7 @@ module Make = (ReconcilerImpl: Reconciler) => { | |||
| /* Only both replacing node if the primitives are different */ | |||
| switch (newInstance.component.element, i.component.element) { | |||
| | (Primitive(newPrim), Primitive(oldPrim)) => | |||
| if (oldPrim != newPrim) { | |||
| if (oldPrim !== newPrim) { | |||
There was a problem hiding this comment.
This is probably the most relevant change. Because we were using structural equality before, when the primitives would contain any function in side (like, say, a Button that has a callback onPress) an exception with Invalid_argument equal: functional value would be thrown.
I am not sure about the perf implications. This change will lead to the reconciler rendering much more often but on the other hand the checks should be much faster too.
There was a problem hiding this comment.
Ah interesting, thanks for calling it out! Would've been easy to miss 😄
Because we were using structural equality before, when the primitives would contain any function in side (like, say, a Button that has a callback onPress) an exception with Invalid_argument equal: functional value would be thrown.
Good catch! Yes, that makes sense that it is problematic in this case. Comparing 'functions' is not really doable in the OCaml sense like it is in JS.
I am not sure about the perf implications. This change will lead to the reconciler rendering much more often but on the other hand the checks should be much faster too.
Yes, good point, I suspect it will depend on how much work it is to create a new 'node' too. Would really need benchmarks to decide if it is a bottleneck or not.
But I'm OK with this going in since it behaves correctly and all the tests are green. We can focus on addressing the performance once we find bottlenecks / have benchmarks to examine. I created #37 to examine this in more depth.
| @@ -0,0 +1,245 @@ | |||
| /** HooksUseReducer **/ | |||
There was a problem hiding this comment.
These tests are mostly adapted from the useState ones.
| | Div | ||
| | Span(string) | ||
| | Image(imageProps); | ||
| | View |
There was a problem hiding this comment.
Exploring the feasibility of making the DOM reconciler primitives more similar to those in Revery 😄
| (state: t, currentVal: 'a) => { | ||
| let updatedVal: ref(Object.t) = ref(Object.to_object(currentVal)); | ||
| state.newState = List.append(state.newState, [updatedVal]); | ||
| let ret: updateFunction('a) = | ||
| let ret: getterAndUpdater('a) = ( | ||
| () => Object.of_object(updatedVal^), |
There was a problem hiding this comment.
I am not 100% sure this is necessary... but I added it just to be sure, for cases when there are two actions being dispatched in the same render.
|
Ah, forgot the gifs!! ✨
|
| validateStructure(rootNode, expectedStructure); | ||
| }); | ||
|
|
||
| test("useReducer can update multiple times", () => { |
There was a problem hiding this comment.
Excellent test coverage @jchavarri - thanks for exercising the new functionality so thoroughly! 💯
| let useState = initialState => { | ||
| let (componentState, dispatch) = | ||
| useReducer(useStateReducer, initialState); | ||
| let setState = newState => dispatch(SetState(newState)); |
There was a problem hiding this comment.
Cool, I like how you implemented useState with the useReducer primitive 👍
|
Woah it's awesome to see it in action with the gifs, so cool! Changes look great to me. I really appreciate the additional tests; gives me a lot of confidence in the change. Bringing it in now! Thanks @jchavarri 💯 |
Fixes #21.
Reactify.re.