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

Hook Reevaluation due to task recreation #3

Open
adam-rocska opened this issue Apr 14, 2023 · 1 comment
Open

Hook Reevaluation due to task recreation #3

adam-rocska opened this issue Apr 14, 2023 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@adam-rocska
Copy link
Owner

As a user of the library it's normal to do this:

      useTaskQueue({
        name: 'test',
        codec: json.number,
        task: v => [v * 2, v * 3, v * 4],
      })

The problem is, that doing so in a react component or a wrapper hook, the task function's reference will always "change" therefore React will assume, that our memorized value should be re-evaluated.

  const task = useMemo(() => descriptor.task, [descriptor.task]);

Gotta somehow fix react's function change detection.

@adam-rocska adam-rocska added the bug Something isn't working label Apr 14, 2023
@adam-rocska adam-rocska self-assigned this Apr 14, 2023
adam-rocska added a commit that referenced this issue Apr 14, 2023
adam-rocska added a commit that referenced this issue Apr 14, 2023
Internally at @21GramConsulting we have stricter
guidelines and smarter automations, but for
OSS this is good enough I guess.

#3
adam-rocska added a commit that referenced this issue Apr 14, 2023
adam-rocska added a commit that referenced this issue Apr 14, 2023
Apparently this issue happens in combination
with NextJS f.e. Anyways, we're good now.

#3
@adam-rocska adam-rocska mentioned this issue Apr 14, 2023
Merged
@adam-rocska
Copy link
Owner Author

Ok, so the problem wasn't memo specifically (though doesn't help), but the useEffect hook listing "idiomatically" its dependencies.

Funnily enough, could only be reproduced in NextJS based environments.

Anyways, now it's supposed to work and we should have gotten rid of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant