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

Update SSR warning message regarding :first-child usage #2325

Open
kidroca opened this issue Apr 1, 2021 · 2 comments
Open

Update SSR warning message regarding :first-child usage #2325

kidroca opened this issue Apr 1, 2021 · 2 comments

Comments

@kidroca
Copy link

kidroca commented Apr 1, 2021

The problem

The advice "Try change it to ":first-of-type" isn't very sound

The pseudo class ":first-child" is potentially unsafe when doing server-side rendering. Try changing it to ":first-of-type".

Developers would blindly follow this advice to suppress the warning (even outside of SSR)

Props

  • the advice covers a lot of the use cases

Cons

  • first-of-type is no substitute for first-child - first-child exists for a reason
  • you might falsely assume it works as you are working on a specific use case
  • you might introduce other elements and the first-child is not of the same type

Proposed solution

Update the warning to either exclude the suggestion and make people think of solution that would be best for them, or
replace Try changing it to ":first-of-type" with something like Would ":first-of-type" cover your use case?

Alternative solutions

An easy way (flag/env/config) to disable this warning for the entire project, so that at least non SSR projects can opt out of the warning

Additional context

I've seen people preferring to suppress this warning by using first-of-type instead of an inline comment to avoid code review problems like "Why have you suppressed ... for this statement"

@iChenLei
Copy link
Contributor

iChenLei commented Apr 8, 2021

Previous discussion

#1178
trendmicro-frontend/tonic-ui#253

@Andarist How about this feature request ? If you have a clear conclusion, I am happy to implement this feature

@Andarist
Copy link
Member

Andarist commented Apr 8, 2021

Sure, please prepare a PR for this and we'll bikeshed the exact text of the message then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants