GoogleChrome / lighthouse-stack-packs

Lighthouse Stack Packs
Apache License 2.0
204 stars 74 forks source link

React stack pack #31

Open housseindjirdeh opened 5 years ago

housseindjirdeh commented 5 years ago

We have an initial set of React-specific suggestions! However, we would love to have more members from the community review and add more if possible.

  1. Do the already populated performance audit suggestions look good? Should they be changed in any way?
  2. For all the remaining performance audits (currently blank), can we add a React-specific suggestion that would be relevant to developers?

All thoughts, comments and PRs appreciated!

djirdehh commented 5 years ago

This is great. How about a suggestion for the use of the useEffect() Hook to avoid having the effect from running on every render?

--> https://reactjs.org/docs/hooks-effect.html#tip-optimizing-performance-by-skipping-effects

{
  // ...,
  skips_applying_effect: If you are using the useEffect() Hook, ensure that you optimize performance by skipping the effect only until certain dependencies have changed. [Learn more](https://reactjs.org/docs/hooks-effect.html#tip-optimizing-performance-by-skipping-effects)
}
benschwarz commented 4 years ago

This is a great start @housseindjirdeh

A couple of questions to get the ball rolling:

† (For the naysayers: As long as it's server-side rendered (and embedded), it's always going to be faster than traditional CSS approaches because the page will only include the CSS it requires.).

housseindjirdeh commented 4 years ago

@djirdehh Ooh good call. With stack packs, we can only extend existing audits (can't add a new skips_applying_effect one), but I think we can definitely include that mention into an already existing description 🚀

housseindjirdeh commented 4 years ago

@benschwarz Thanks a ton, these are all great suggestions.

Unused CSS could be used as a suggestion to consider taking up css-in-js?

Although I agree with your footnote, definitely a tricky one. We're trying to not provide directions that are specific but not particularly to React itself. Might be safest to not point users in that direction since many have pretty strong opinions 😅

Unused JS might be a better signal for suggesting code splitting?

Good call 👍

Are we able to get introspection to suggest when to use React.PureComponent?

Yep like @djirdehh's suggestion, I think it would be worthwhile to include a mention of the different approaches that can be taken to minimize re-renders. Will try and find a place for it!

What about critical/blocking js?

Definitely would be good to mention preloading/service workers. The main Lighthouse audits suggest these so would be nice if we can mention something more relevant to React here 🤔

peaonunes commented 4 years ago

Is there any way to take advantage of the built in React warnings? The react development mode shows up warns about potential bad practices such as forgetting key attribute in list elements and so on. If that information is not too hard to retrieve then it might be a good case...

Thinking about bad practices that might lead to bugs maybe look up to potential wrong usage. For example, callback functions executed in the render phase instead of just triggered by the action.

<button onClick={callback("fire")} />

I do not know if this goes too much under implementation details I just found myself thinking about it.