Skip to content

Conversation

@psychedelicious
Copy link
Contributor

@psychedelicious psychedelicious commented Aug 16, 2023

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization / techdebt
  • Documentation Update
  • Community Node Submission

Description

Add some eslint rules to support clear and performant code.

curly requires conditionals to use curly braces

Conditionals without curly braces like if (thing) doOtherThing() are easy to screw up. No longer allowed.

react/jsx-curly-brace-presence requires string props to not have curly braces

<MyComponent label={'Some Text'} /> is no longer allowed. Must be <MyComponent label='Some Text' />.

react-memo/require-memo requires function components to be wrapped in React.memo

Yes, every component. React already tracks component props to do its diffing, so this is free. Prop comparing is effectively never a performance concern, while in our complex app with very deeply-nested component hierarchy, rerenders are a huge concern.

react-memo/require-usememo requires all complex props (objects, functions) to be wrapped in useMemo or useCallback

Yes, every object or function.

There is a slight upfront cost to this, but using React.memo is negated if all props are not also memoized.

Memoizing everything

The last two are perhaps controversial.

Needing to decide when to use React.memo, React.useMemo and React.useCallback is both unnecessary cognitive overhead and premature optimization.

Using them as a rule is both simpler and will generally lead to better performance. This is relevant to InvokeAI because this app is fairly complex, with deeply-nested components and a crapload of them.

So from now on, everything should be memoized.

Teeny tiny problem

But, uh, there's a minor issue... with these rules enabled, we have a bit of work to do:
image

Soooooo I'm going to leave this here as future-us problem.

- `curly` requires conditionals to use curly braces
- `react/jsx-curly-brace-presence` requires string props to *not* have curly braces
- `react-memo/require-memo` requires function components to be wrapped in `memo`
- `react-memo/require-usememo` requires all complex props (objects, functions) to be wrapped in `useMemo` or `useCallback`
@psychedelicious
Copy link
Contributor Author

superseded by #4306

I added the curly braces and jsx quotes rules in there.

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.

3 participants