Do not alter paths data when loading through d3js #7026
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hazzadous
reviewed
Nov 10, 2021
| @@ -271,7 +271,10 @@ export function NewPaths({ dashboardItemId = null, color = 'white' }: PathsProps | |||
|
|
|||
| const svg = createCanvas(width, height) | |||
| const sankey = createSankey(width, height) | |||
| const { nodes, links } = sankey(paths) | |||
| const { nodes, links } = sankey({ | |||
| nodes: paths.nodes.map((d) => ({ ...d })), | |||
There was a problem hiding this comment.
non-blocking
Should we add in ...paths as well to ensure that any other attributes that were considered and part of paths are also considered by sankey. I have no idea if that's a likely scenario
There was a problem hiding this comment.
Reading the code in d3-sankey I'm pretty sure it's only node and links that are used by sankey
(We also explode paths the same way in the Old Paths.tsx)
There was a problem hiding this comment.
sankey code here https://github.com/d3/d3-sankey/blob/master/src/sankey.js
There was a problem hiding this comment.
and our older usage here https://github.com/PostHog/posthog/blob/master/frontend/src/scenes/paths/Paths.jsx#L139
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
closes #7021 and #6934
How did you test this code?
by running it locally and showing I can save the data with this fix in place