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

[Feature request?] Memoizing object and array literals passed to React properties, child array mapping #1

Open
jedwards1211 opened this issue Mar 1, 2019 · 2 comments

Comments

@jedwards1211
Copy link

I wanted to check if this plugin already does things like this?

Before

const Foo = ({siteId, configId}) => (
  <Plot
    channelIds={[
      `site/${siteId}/simulation/${configId}/inverter/pv_load`,
      `site/${siteId}/simulation/${configId}/grid/building_load`,
    ]}
  />
)

After

const Foo = ({siteId, configId}) => (
  <Plot
    channelIds={useMemo(() => [
      `site/${siteId}/simulation/${configId}/inverter/pv_load`,
      `site/${siteId}/simulation/${configId}/grid/building_load`,
    ], [siteId, configId])}
  />
)

Also children can benefit from memoization too:

Before

const Plot = ({channelIds}) => (
  <div>
    {channelIds.map(channelId =>
      <Trace key={channelId} channelId={channelId} />
    )}
  </div>
)

After

const Plot = ({channelIds}) => (
  <div>
    {useMemo(() => channelIds.map(channelId =>
      <Trace key={channelId} channelId={channelId} />
    ), [channelIds])}
  </div>
)
@jedwards1211 jedwards1211 changed the title Memoizing object and array literals passed to React properties, child array mapping [Feature request?] Memoizing object and array literals passed to React properties, child array mapping Mar 1, 2019
@DAB0mB
Copy link
Owner

DAB0mB commented Mar 5, 2019

I'm just trying to think, because it seems like this behavior might break very easily. What if there's a condition expression? Such as cond ? a : b. The order of the hooks is essential with React hooks. I hope I'm not wrong about it. Feel free to debunk my theory

@jedwards1211
Copy link
Author

jedwards1211 commented Mar 5, 2019

Ah, I'm sorry, I inlined the useMemo calls in my examples just to demonstrate where the memoization is being applied, and you are right, if the parent components were rendered conditionally it would cause problems. Definitely in a real implementation we would want to hoist the memoization up to the top, so for the first example:

const Foo = ({siteId, configId}) => {
  const _channelIds = useMemo(() => [
    `site/${siteId}/simulation/${configId}/inverter/pv_load`,
    `site/${siteId}/simulation/${configId}/grid/building_load`,
  ], [siteId, configId])
  return <Plot channelIds={_channelIds} />
}

As long as the hooks are hoisted there are no naive rules of hooks violations I can think of...

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

2 participants