Skip to content

solid/reactivity should treat functions in mergeProps as being inside a tracking context #179

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

Open
1 task done
fongandrew opened this issue Feb 24, 2025 · 1 comment
Open
1 task done
Assignees
Labels
bug Something isn't working

Comments

@fongandrew
Copy link

Describe the bug
If you put a function inside mergeProps that calls a signal or prop, solid/reactivity will warn:

This function should be passed to a tracked scope (like createEffect) or an event handler because it contains reactivity, or else changes will be ignored.

However, this is not true because mergeProps implicitly wraps all functions with a createMemo. You can verify that reactivity still happens here: https://playground.solidjs.com/anonymous/7998ef80-905a-485a-b7d2-9f7f4f4212a6

To Reproduce
Put a function calling a signal inside mergeProps:

const props = mergeProps(props, () => ({ otherProp: signalAccessor() }));

Expected behavior
There is no warning.

Screenshots
https://github.com/user-attachments/assets/a5cad8e7-3e0e-428f-ad1b-2c60435b66a0

Environment (please complete the following information):

  • OS: MacOS 15.3.1
  • Node version (node --version): 23.5.9
  • eslint-plugin-solid version (npm list eslint-plugin-solid/yarn why eslint-plugin-solid): 0.14.4
  • eslint version (npm list eslint/yarn why eslint): 9.16.0

Additional context

  • I would be willing to contribute a PR to fix this issue (happy to dig in but this issue is also my way of getting a second opinion that functions in mergeProps are, in fact, safe to treat as tracking).
@fongandrew fongandrew added the bug Something isn't working label Feb 24, 2025
@fongandrew
Copy link
Author

FWIW, adding mergeProps to customReactiveFunctions seems to work as a workaround.

@fongandrew fongandrew changed the title solid/reactivity should treat functions in mergeProps as being inside a track solid/reactivity should treat functions in mergeProps as being inside a tracking context Feb 24, 2025
fongandrew added a commit to fongandrew/janus-ui that referenced this issue Mar 17, 2025
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

2 participants