Skip to content

Feature Request: Provide an optimization option for simple stateless components #20564

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

Closed
bglgwyng opened this issue Jan 9, 2021 · 6 comments

Comments

@bglgwyng
Copy link

bglgwyng commented Jan 9, 2021

Please look at the following code first.

function A() {
  return <HeavyComponent someProp={true} />;
}

function B() {
  return <HeavyComponent someProp={false} />;
}

function C() {
  const [state, setState] = useState(true);
  return state ? <A /> : <B />;
}

function D() {
  const [state, setState] = useState(true);
  return state ? A() : B();
}

HeavyComponent is literally a heavy component that requires large computation for rendering. Assume that nobody wants the scenario that HeavyComponent mounts and unmounts frequently.

And as you can see, A and B do very simple jobs. They are just tiny components that just wrap HeavyComponent in each different way. A and B can be switched to each other in both C and D. The rendering result will be the same in C and D. But the internal behaviors differ from each other. C causes HeavyComponent to mount whenever it rerenders, while D doesn't. If there are useEffect calls with empty deps inside HeavyComponent's body, then they will be called each time state in C changes, while they won't with D. So, D is better in performance than C.

However, the problem is that A() and B() are not the recommended style in React code. If A and B are used only inside the module not being exported to others than the author, then A() is an acceptable hack. But sometimes, at least in my case, components like A and B should be exported outside the module. And it's hard to expect users of my library to use A and B in an efficient way like inside D. I hope there is a way to let users use <A /> instead of A() not worrying about the performance.

I think you can point out that I don't need to define A and B and I can use HeavyComponent directly instead. But it's just a simplified example. In real practice, I have more than one props and also more components. Each component represents the semantic of the combination of props passing to HeavyComponent by name of itself.

My case is a little specific and not usual, so I don't think I can explain well my issue. I'll happy if someone helps me with familiar examples of A and B in real practice that many people have met before.

I'll also export HeavyComponent directly, but users will get confused about its props at the first glance. So it would be better to export handy components A and B also.

I suppose the implementation would not be complicated. The following is my psuedo code.

function simpleComponent(component) {
  const x = (...args) => component(...args)
  x.isSimple = true;
  return x;
}

function createElement(component, props) {
  if(component.isSimple) {
    return component(props);
  }
  // do original behavior
  // ...
}

const A = simpleComponent(() => <HeavyComponent someProp={true} />);
const B = simpleComponent(() => <HeavyComponent someProp={false} />);

I said stateless in the title, but hookless would be the correct condition. useEffect, useRef and other hooks are not permitted inside the body of simple components. Of course, I don't think simple is a good name. I'll be happy to see suggestions.

Does this seem a reasonable option to consider?

@rickhanlonii
Copy link
Member

Hey @bglgwyng, thanks for taking time to write out this proposal.

Isn't this what React.memo does already?

@bglgwyng
Copy link
Author

React.memo can't prevent unmount-and-mount of HeavyComponent.

const HeavyComponent = React.memo(() => {
  useEffect(() => { console.log("mount!") }, []);
  return <span>hi</span>
});

If HeavyComponent is implemented in this way, every time state in C changes, we'll see "mount!" message.

@urugator
Copy link

Why not to simply expose functions like renderA and renderB instead?

Or you can expose default props for each variant:

function Cmp() {
  const defaultProps = state ? defaultPropsA : defaultPropsB
  const props = {
    ...defaultProps,
    customProp: "foo"
  }
  return <HeavyComponent {...props}>
}

Or pass a prop that denotes specific variant

function HeavyComponentOfType({ type, ...props }) {
   switch (type) {
	case "A": return (<HeavyComponent someProp="true" {...props}>);
	case "B": return (<HeavyComponent someProp="false" {...props}>);
	default: (() => throw new Error('Unknown type'))()
   }
}

@bvaughn
Copy link
Contributor

bvaughn commented Jan 11, 2021

@rickhanlonii This issue is a variation of a request for reparenting support (#3965). Some components (subtrees) are expensive to create/destroy but there's no way to move them around other than destroying and recreating (and also lifting up/restoring any state that goes along with them).

I'm not sure the proposed mechanism makes sense to me though. In the very simplified example given, the proposal would be overkill obviously– and I'm not sure if it would make sense in a larger, more "real world app" context. Definitely seems like something that would need to go through our RFC process regardless: https://github.com/reactjs/rfcs#react-rfcs

That being said, the RFC process moves very slowly due to our small team size.

@bvaughn
Copy link
Contributor

bvaughn commented Jan 12, 2021

I'm going to tag this as "discussion" but I don't think it's a complete enough proposal for much of a detailed discussion to take place. I still think it should be an RFC instead. That process kind of requires thinking through pros and cons a little more.

That being said, for a discussion to be fruitful here, I think we need at least one more realistic example. The original example shared doesn't really make much sense and so the proposal would not be needed. I'm not convinced that the proposal would make sense in a larger, more real-world application either, but maybe we could start off by looking at something more representative of one.

@bvaughn
Copy link
Contributor

bvaughn commented Mar 22, 2021

It's been a couple of months and the discussion seems to have dead ended without any additional examples or use cases, so I'm going to close this issue.

@bvaughn bvaughn closed this as completed Mar 22, 2021
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

4 participants