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

React stack pack #31

Open
housseindjirdeh opened this issue Oct 1, 2019 · 5 comments
Open

React stack pack #31

housseindjirdeh opened this issue Oct 1, 2019 · 5 comments

Comments

@housseindjirdeh
Copy link
Contributor

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
Copy link

djirdehh commented Oct 1, 2019

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
Copy link

benschwarz commented Oct 4, 2019

This is a great start @housseindjirdeh

A couple of questions to get the ball rolling:

  • Unused CSS could be used as a suggestion to consider taking up css-in-js? †
  • Unused JS might be a better signal for suggesting code splitting?
  • Are we able to get introspection to suggest when to use React.PureComponent?
  • What about critical/blocking js? If someone builds a single page app that isn't server side renders and there's a critical path to app.js + vendor.js, should we suggest to preload those resources? OR should we suggest service workers so that the bytecode is cached?

† (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
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link

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.

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

4 participants