Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

Fix setProps when IDs aren't provided #166

Merged
merged 7 commits into from
Apr 21, 2019
Merged

Fix setProps when IDs aren't provided #166

merged 7 commits into from
Apr 21, 2019

Conversation

chriddyp
Copy link
Member

@chriddyp chriddyp commented Apr 19, 2019

Fixes #165

construct paths on-the-fly while recursing through the tree.
@chriddyp chriddyp changed the title Fix setProps when IDs aren Fix setProps when IDs aren't provided Apr 19, 2019
@chriddyp chriddyp changed the base branch from master to qa April 19, 2019 21:19
@chriddyp chriddyp requested review from byronz and alexcjohnson April 19, 2019 21:58
addIndex(map)(
(component, i) => createContainer(component, concat(path, ['props', 'children', i])),
components
) : createContainer(components, concat(path, ['props', 'children']));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Non-🚫 but to my eye ramda makes this much harder to read, vs vanilla

components.map((component, i) =>
    createContainer(component, concat(path, ['props', 'children', i]))
)

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Looks awesome! This is hardly any extra code, can't imagine it being much of a perf drag - nicely done! 💃

@alexcjohnson
Copy link
Collaborator

@chriddyp this is awesome (my 💃 stands BTW, despite the extra discussion). But @byronz merged some of the if(id) hacks in plotly/dash-core-components#523, do we need to remove them now? Seems like they could actually be harmful once this PR is merged.

@chriddyp
Copy link
Member Author

Yeah - I think I'll follow up with a PR that cleans up some of the DCC logic. I think that the id => setState is actually still "safe" but it could be cleaned up - many of the components can become functional & stateless

@chriddyp chriddyp merged commit 45136e2 into qa Apr 21, 2019
@chriddyp chriddyp deleted the paths branch April 21, 2019 01:29
@chriddyp chriddyp mentioned this pull request Apr 22, 2019
1 task
chriddyp added a commit that referenced this pull request Apr 22, 2019
provide valid setProps for all components, even if they don't have an ID

construct paths on-the-fly while recursing through the tree.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants