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

Disallow duplicate component ids. #320

Merged
merged 3 commits into from
Aug 13, 2018
Merged

Disallow duplicate component ids. #320

merged 3 commits into from
Aug 13, 2018

Conversation

T4rk1n
Copy link
Contributor

@T4rk1n T4rk1n commented Aug 2, 2018

Fix #299.
Raise DuplicateIdError if there is more than one component with the same id before rendering the first request.

@T4rk1n T4rk1n requested review from chriddyp and rmarren1 August 7, 2018 23:19
@T4rk1n
Copy link
Contributor Author

T4rk1n commented Aug 7, 2018

I have encountered this error, it was really hard to debug for something this simple when you have a complex layout.

@rmarren1
Copy link
Contributor

rmarren1 commented Aug 8, 2018

💃

I think it is still possible to get duplicate IDs if you change the ID of a component using a callback. This is probably very bad practice and nobody should be doing it, but maybe we could just change the error message to 'Duplicate component id found in initial layout : {}'.format(component_id)) to be clear we aren't checking for that.

@T4rk1n T4rk1n force-pushed the duplicate-component-ids branch from c533b49 to 14e8308 Compare August 9, 2018 21:22
@chriddyp
Copy link
Member

💃

@T4rk1n T4rk1n force-pushed the duplicate-component-ids branch from 14e8308 to 2d8a033 Compare August 13, 2018 16:42
@T4rk1n T4rk1n merged commit a5b7592 into master Aug 13, 2018
@T4rk1n T4rk1n deleted the duplicate-component-ids branch August 13, 2018 16:49
AnnMarieW pushed a commit to AnnMarieW/dash that referenced this pull request Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants