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

[HEAP-42725] Return the same component each time from withHeapNavigationWrapper #406

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

bnickel
Copy link
Collaborator

@bnickel bnickel commented Sep 13, 2023

Description

If Heap.withHeapNavigationWrapper is called multiple times, it currently returns a new HOC each time. This means if the app looks like this:

const App = () => {

  ...

  const HeapNavigationContainer =
    Heap.withReactNavigationAutotrack(NavigationContainer);

  return (
    <HeapNavigationContainer independent={true}>
      ...
    </HeapNavigationContainer>
  );
};

And the app contains a useEffect or similar, which caused App to re-evaluate, the entire contents of HeapNavigationContainer will be considered new and will be recreated, sending the app back to start.

The general fix is to move Heap.withHeapNavigationWrapper out of a function but this has come up enough that a code fix is warranted.

This simply caches the output of withReactNavigationAutotrack on the wrapped component so it can be retrieved the next time it is called.

Test Plan

Unit tests were added and executed. This behavior was vetted in a test app.

It would be possible to write an integration test on this, if someone wants to.

Checklist

  • Detox tests pass
  • If this is a bugfix/feature, the changelog has been updated

…tionWrapper`

If `Heap.withHeapNavigationWrapper` is called multiple times, it currently returns a new HOC each time.  This means if the app looks like this:

```
const App = () => {

  ...

  const HeapNavigationContainer =
    Heap.withReactNavigationAutotrack(NavigationContainer);

  return (
    <HeapNavigationContainer independent={true}>
      ...
    </HeapNavigationContainer>
  );
};
```

And the app contains a `useCallback` or similar, which caused `App` to re-evaluate, the entire contents of `HeapNavigationContainer` will be recreated, sending the app back to start.

The general fix is to move `Heap.withHeapNavigationWrapper` out of a function but this has come up enough that a code fix is warranted.

This simply caches the output of `withReactNavigationAutotrack` on the wrapped component so it can be retrieved the next time it is called.
Copy link
Collaborator

@MCeley MCeley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link

@azelaya6 azelaya6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@bnickel bnickel merged commit 1e1edc3 into develop Sep 26, 2023
@bnickel bnickel deleted the one-appcontainer-one-wrapper branch September 26, 2023 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants