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

Hook injection makes certain styles not being applied properly #752

Closed
danqing opened this issue Oct 10, 2021 · 10 comments
Closed

Hook injection makes certain styles not being applied properly #752

danqing opened this issue Oct 10, 2021 · 10 comments

Comments

@danqing
Copy link

danqing commented Oct 10, 2021

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

CleanShot 2021-10-09 at 23 30 51

On page load certain css styles aren't being loaded properly.

If the current behavior is a bug, please provide the steps to reproduce and possibly a minimal demo or testcase in the form of a Next.js app, CodeSandbox URL or similar

A repo has been uploaded here: https://github.com/luma-team/styled-jsx-issue. Run the code and you can see the above symptom.

What is the expected behavior?

The page loads to the end state directly.

Environment (include versions)

All OS / browser should have the issue.

Did this work in previous versions?

Yes.


Some additional info:

  1. There's some async loading involved, which means that the client-side final state is different from the server-rendered version.
  2. In the repro, input's autofocus is triggering the issue. If you take out the autofocus, it works ok. I think there are other ways to trigger the issue in our app, though I'm not sure what they are.
  3. As you can see, the elements have a transition attribute, which is why on page load they have the funny animation. But when an element first renders, the transition shouldn't kick in (and it didn't in previous versions).

Let me know if there are other information I can provide. Thank you!

@danqing
Copy link
Author

danqing commented Oct 16, 2021

Any thoughts @huozhi ? :)

@huozhi
Copy link
Member

huozhi commented Oct 16, 2021

Thanks for reporting, the inconsistence is caused by same property chaing from one in preloaded css (bootstrap.css in your example) and then changed in styled-jsx css so that it would trigger a css transition.

Previously style tags are injected in render phase of React but it's highly dangerous and will lead to more inconsistence when upgrading to React 18 under concurrent mode. In this short period we don't have a good solution/hook to inject CSS ealier.

There's still a workaround that you can try to create the styles only by styled-jsx for a component instead of mixing preloaded CSS styles and dynamic injected styles together.

@danqing
Copy link
Author

danqing commented Oct 16, 2021

Thank you @huozhi.

Why is it dangerous to inject styles in render phase? Isn't <style> exactly something the browser needs to render? The glitch is because the DOM is being rendered first (also in JSX) and then the <style jsx> tag gets added to it. This means that styles will always be applied later, and so some other style will be there instead (whether it's some global styles or browser default styles).

CSS feels like something that should always be present when rendering..

@huozhi
Copy link
Member

huozhi commented Oct 17, 2021

All side effects are potentially dangerous.

In async rendering the render might be called multiple times which might bring more side effects. So that's why we rely on hooks for safety now. Current implementation is a transition from synchronized rendering to the future asynchronized rendering, React will propbably solve this more elegantly in the future.

@huozhi
Copy link
Member

huozhi commented Oct 29, 2021

Upgrading to react 18 (currently in alpha) and next 12 can actually resolve this problem. Since we introduced the new hook from react for styled-jsx. I think you can wait for react 18 out or you can try it in advance. I'll close the issue since this might be the way to go for solving it without workaround for your case.

@huozhi huozhi closed this as completed Oct 29, 2021
@danqing
Copy link
Author

danqing commented Oct 29, 2021

Thank you @huozhi!

@danqing
Copy link
Author

danqing commented Apr 29, 2022

@huozhi this is somehow broken again on 5.0.2. I can confirm that 5.0.1 works fine. a bit unexpected because the commits between 5.0.1 and 5.0.2 seem pretty innocuous..

@huozhi
Copy link
Member

huozhi commented Apr 29, 2022

@danqing Did you somehow manually install styled-jsx to your nextjs project? That will lead to FOUC problem where you have 2 styled-jsx registry, the collected styles aren't aigned in that case.

Latest next.js is integrated with 5.0.1, and 5.0.2 will released along with next version.

Can you please run npm ls styled-jsx to see if it's installed twice? I shouldn't require the manual install if you're runing with nextjs

@danqing
Copy link
Author

danqing commented Apr 29, 2022

Ah interesting... yea we do have styled-jsx in our own package.json. Are you suggesting us removing it? nextjs doesn't have styled-jsx as a dependency though, right?

@huozhi
Copy link
Member

huozhi commented Apr 29, 2022

next.js does have styled-jsx as dependency, if you installs next.js, styled-jsx should be able to be used out of box. That's whay I suggested to remove the duplicated one.

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

No branches or pull requests

2 participants