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

Clean up measurements Redux states/actions #1881

Merged
merged 12 commits into from
Nov 6, 2024
3 changes: 2 additions & 1 deletion src/actions/measurements.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,8 @@ const getCollectionDisplayControls = (controls, collection) => {
};

const parseMeasurementsJSON = (json) => {
const collections = json["collections"];
// Avoid editing the original json values since they are cached for narratives
const collections = cloneDeep(json["collections"]);
Copy link
Member

Choose a reason for hiding this comment

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

[aside]

When we're not viewing a narrative we could avoid this and reduce the memory footprint. I don't actually want to do it because it makes testing harder and will invariably result in bugs somewhere, but something that came to mind as I read this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I thought about adding some conditionals around narratives, but like you said, it seemed like that would eventually result in bugs somewhere 🙈

if (!collections || collections.length === 0) {
throw new Error("Measurements JSON does not have collections");
}
Expand Down