-
Notifications
You must be signed in to change notification settings - Fork 504
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
fix(transformData): always call transformData #1555
Conversation
Event when data does not changes, we now call transformData, so that if the transformedData changes, there will be a new render. fixes #1538
By analyzing the blame information on this pull request, we identified @redox, @bobylito and @pixelastic to be potential reviewers |
// Resolve transformData before Template, so transformData is always called | ||
// even if the data is the same. Allowing you to dynamically inject conditions in | ||
// transformData that will force re-rendering | ||
const withTransformData = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the interesting part
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered using alternative implementations? What about use transformData in willReceiveProps and store the result in the state?
@@ -187,17 +179,6 @@ describe('refinementList()', () => { | |||
createURL = () => '#'; | |||
}); | |||
|
|||
it('formats counts', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the place to test this, applied helpers are already tested in Template-test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation is solid, just wondering if we could have not used a HOC for that.
// Resolve transformData before Template, so transformData is always called | ||
// even if the data is the same. Allowing you to dynamically inject conditions in | ||
// transformData that will force re-rendering | ||
const withTransformData = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered using alternative implementations? What about use transformData in willReceiveProps and store the result in the state?
Indeed this complexifies a bit the code, => Not worth investigating in the end, after discussing with @bobylito. |
<a name="1.8.15"></a> ## [1.8.15](v1.8.14...v1.8.15) (2016-11-16) ### Bug Fixes * **priceRanges:** avoid displaying solo ranges (#1544) ([ff396f0](ff396f0)), closes [#1536](#1536) * **priceRanges:** use formatNumber in defaultTemplate (#1559) ([557a501](557a501)), closes [#1230](#1230) * **toggle:** support negative numeric values for on/off (#1551) ([e4d88e0](e4d88e0)), closes [#1537](#1537) * **transformData:** always call transformData (#1555) ([49bfeca](49bfeca)), closes [#1538](#1538)
This reverts commit 49bfeca.
Event when data does not changes, we now call transformData, so that if the transformedData changes, there will be a new render. fixes #1538
* fix(transformData): always call transformData (#1555) Event when data does not changes, we now call transformData, so that if the transformedData changes, there will be a new render. fixes #1538 * fix(transformData): default data is an object when not provided This fixes the nasty regression bug we have seen and reimplement the transformData fix
Summary:
Even when data does not changes, we now call transformData. So that
if the transformed data changes, there will be a new render call.
fixes #1538
linked to #1100