search

Templarian / MaterialDesign-React

Dist for Material Design Icons React Component for JS/TypeScript
https://materialdesignicons.com
MIT License
139 stars 20 forks source link

Incremented `id` doesn't play nicely w/ SSR #50

Closed mAAdhaTTah closed 3 years ago

mAAdhaTTah commented 3 years ago

Incrementing the id will cause HTML mismatch warnings when SSR'd, because the id incrementing will occur in a single context (the node server) when generating the server HTML but occur in "multiple" contexts (each user's browser) when hydrated. To put it another way, if you get 3 requests, it'll give id 1 to the first, id 2 to the second, id 3 to the third, but each will get id 1 when the client does its rendering, because it doesn't load up with the same id as present on the server (cuz it's starting from scratch, so to speak).

Easiest non-breaking solution would be to allow id to be provided as a prop and fall back to the incrementing if no id is provided. I'd be happy to open a PR to get this fixed if it would be accepted, as it's causing some issues with our Next.js project.

Thanks for the icon suite! It's been really helpful.

Templarian commented 3 years ago

@mAAdhaTTah Is there a convention for this prop name possibly you've seen elsewhere. Definitely would take a PR for this.

I'm assuming a lot of components run into this if you can find an example somewhere else that will make me feel better about the change.

mAAdhaTTah commented 3 years ago

@Templarian Personally, I think id would be fine, since that's the underlying attribute. For other examples, we ran into a similar issues with Downshift, which uses id and will generate a default value if one isn't provided.

Templarian commented 3 years ago

@mAAdhaTTah It looks like they try to solve this issue with useOpaqueIdentifier. I'm fine merging a PR that takes in an id.

Templarian commented 3 years ago

Going to re-open this until I have it published to NPM.

@mAAdhaTTah Annoy me if this is not published by Saturday afternoon please.

mAAdhaTTah commented 3 years ago

@Templarian Will do, thank you!

mAAdhaTTah commented 3 years ago

@Templarian Hope you're having a great weekend! Obvs I didn't tag you on this last weekend but maybe you have some time this one?

shabith commented 3 years ago

Hi @Templarian any update on this? seems like it is not published to NPM yet.

Thanks @mAAdhaTTah for your PR on this.

Templarian commented 3 years ago

This was published @mAAdhaTTah @shabith! I really need to setup something that reminds me of this stuff. An AI to better manage my open source stuff. Like "That one repo that you never check has stuff."

Sorry about that!

bdemirel commented 2 years ago

I was facing the same problem and took me some time to find this issue/PR. Thanks for the fix, but it may be useful to add a note about this on the website!