-
Notifications
You must be signed in to change notification settings - Fork 525
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
Optimized stringMap calculation #1211
Optimized stringMap calculation #1211
Conversation
|
||
const initialMemo = { x: [], y: [] }; | ||
const combine = (memo, datum) => ({ | ||
x: datum.x ? memo.x.concat(datum.x) : memo.x, |
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.
I think these should be datum.x !== undefined ?
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.
Though maybe this is unnecessary since the case where these would be empty strings is a bit contrived.
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.
!== undefined
is better, I'll change that.
sorry about the timing. We just added prettier. Would you mind rebasing:
|
Happy New Year! These changes are desirable. Do you have a moment to rebase? |
88bc033
to
5258110
Compare
Sorry for being so unresponsive, but I was on a vacation over chrismas. The rebase to prettier is now done. |
🎉 Thanks! |
This is the optimization I spoke of in a recent issue (#1207).
Instead of iterating (including formatting, cleaning et.) the data twice and extracing a different property each time, the data is iterated once and both properties are extracted.
To achive this I had to adapt
reduceChildren
to allow different kinds of reduce functions instead of accumulating every result in an array, otherwise I would have ended up with an array of objects containing thex
andy
properties, which would have required another iteration for unwrapping.