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

[@mantine/core] Image: fix possible permanent placeholder after hydration #630

Merged
merged 1 commit into from
Jan 8, 2022

Conversation

Stouffi
Copy link
Contributor

@Stouffi Stouffi commented Jan 7, 2022

This PR fixes #629

@@ -1 +1 @@
v14.9.0
v14.17.0
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason to change node version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was stuck with an error stating that a dependency required node >= 14.17 during my first yarn install

Copy link
Member

Choose a reason for hiding this comment

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

Alright, then let's leave it in 14.17 then

@rtivital
Copy link
Member

rtivital commented Jan 7, 2022

Thanks for a PR, have you tested it to work in ssr environment, e.g. in next template?

@Stouffi
Copy link
Contributor Author

Stouffi commented Jan 7, 2022

@rtivital I encountered the issue with my stack which is viteJS + SSR and with the fix, the problem with placeholder disappeared.

@rtivital
Copy link
Member

rtivital commented Jan 7, 2022

Okay, I'll run some more tests with it then

@Stouffi
Copy link
Contributor Author

Stouffi commented Jan 7, 2022

Oh btw, did you mean testing manually with other SSR environment? I couldn't figure out how to add a test with an html string and an hydration using @testing-library/react in src/mantine-core/src/components/Image/Image.test.tsx

@rtivital
Copy link
Member

rtivital commented Jan 7, 2022

Yes, I do not think it is possible to check it within jest-dom environment, so it should be checked manually at least in Next.js

@rtivital rtivital merged commit f7b9c5b into mantinedev:master Jan 8, 2022
@rtivital
Copy link
Member

rtivital commented Jan 8, 2022

I've checked with next, it works fine

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.

Race condition between hydration and image load event
2 participants