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

@preact/signals-react 1.3.6 is brokes useSyncExternalStoreWithSelector #411

Open
XantreDev opened this issue Sep 10, 2023 · 16 comments
Open
Labels

Comments

@XantreDev
Copy link
Contributor

I am using @tanstack/react-router which using useSyncExternalStoreWithSelector i've worked with this combo for week, but yesterday it started to throw inside react internals. I has really long research and fixed this issue by downgrading to 1.2.2 where was no react internals patching. I dont really know how to reproduce this issue, maybe it's because signals uses sync external store too

react-dom.development.js:16249 Uncaught TypeError: Cannot read properties of undefined (reading 'length')
    at areHookInputsEqual (react-dom.development.js:16249:38)
    at updateMemo (react-dom.development.js:17240:11)
    at Object.useMemo (react-dom.development.js:17886:16)
    at useMemo (react.development.js:1650:21)
    at useSyncExternalStoreWithSelector (with-selector.development.js:64:18)
    at useStore (index.tsx:17:17)
    at useRouterState (react.tsx:568:10)
    at useMatch (react.tsx:673:24)
    at useSearch (react.tsx:802:10)
    at _Task (Task.tsx:23:57)
areHookInputsEqual @ react-dom.development.js:16249
updateMemo @ react-dom.development.js:17240
useMemo @ react-dom.development.js:17886
useMemo @ react.development.js:1650
useSyncExternalStoreWithSelector @ with-selector.development.js:64
useStore @ index.tsx:17
useRouterState @ react.tsx:568
useMatch @ react.tsx:673
useSearch @ react.tsx:802
_Task @ Task.tsx:23
renderWithHooks @ react-dom.development.js:16305
updateFunctionComponent @ react-dom.development.js:19588
updateSimpleMemoComponent @ react-dom.development.js:19425
beginWork @ react-dom.development.js:21678
callCallback2 @ react-dom.development.js:4164
invokeGuardedCallbackDev @ react-dom.development.js:4213
invokeGuardedCallback @ react-dom.development.js:4277
beginWork$1 @ react-dom.development.js:27451
performUnitOfWork @ react-dom.development.js:26557
workLoopSync @ react-dom.development.js:26466
renderRootSync @ react-dom.development.js:26434
performSyncWorkOnRoot @ react-dom.development.js:26085
flushSyncCallbacks @ react-dom.development.js:12042
(anonymous) @ react-dom.development.js:25651
setTimeout (async)
(anonymous) @ utils.ts:405
sleep @ utils.ts:404
scheduleMicrotask @ utils.ts:414
flush @ notifyManager.ts:64
batch @ notifyManager.ts:31
dispatch @ query.ts:590
setData @ query.ts:204
onSuccess @ query.ts:472
resolve @ retryer.ts:103
Promise.then (async)
run @ retryer.ts:153
createRetryer @ retryer.ts:204
fetch @ query.ts:458
executeFetch @ queryObserver.ts:350
onSubscribe @ queryObserver.ts:107
subscribe @ subscribable.ts:15
(anonymous) @ useBaseQuery.ts:81
subscribeToStore @ react-dom.development.js:16958
commitHookEffectListMount @ react-dom.development.js:23150
commitPassiveMountOnFiber @ react-dom.development.js:24926
commitPassiveMountEffects_complete @ react-dom.development.js:24891
commitPassiveMountEffects_begin @ react-dom.development.js:24878
commitPassiveMountEffects @ react-dom.development.js:24866
flushPassiveEffectsImpl @ react-dom.development.js:27039
flushPassiveEffects @ react-dom.development.js:26984
commitRootImpl @ react-dom.development.js:26935
commitRoot @ react-dom.development.js:26682
performSyncWorkOnRoot @ react-dom.development.js:26117
flushSyncCallbacks @ react-dom.development.js:12042
(anonymous) @ react-dom.development.js:25651
Show 52 more frames
Show less
react-dom.development.js:16249 Uncaught TypeError: Cannot read properties of undefined (reading 'length')
    at areHookInputsEqual (react-dom.development.js:16249:38)
    at updateMemo (react-dom.development.js:17240:11)
    at Object.useMemo (react-dom.development.js:17886:16)
    at useMemo (react.development.js:1650:21)
    at useSyncExternalStoreWithSelector (with-selector.development.js:64:18)
    at useStore (index.tsx:17:17)
    at useRouterState (react.tsx:568:10)
    at useMatch (react.tsx:673:24)
    at useSearch (react.tsx:802:10)
    at _Task (Task.tsx:23:57)
@diego9497
Copy link

I'm having the same issue, I'm using @preact/signals-react@1.3.4, @reduxjs/toolkit"@1.8.5 and react@17.0.2, downgrade doesn't work for me since start thrown errors with the router.

react-dom.development.js:15866 Uncaught Error: TypeError: Cannot read properties of undefined (reading 'length')
    at updateMemo (react-dom.development.js:15866:1)
    at Object.useMemo (react-dom.development.js:16422:1)
    at useMemo (react.development.js:1532:1)
    at useSyncExternalStoreWithSelector (with-selector.development.js:64:1)
    at useSelector (useSelector.js:41:1)
    at TaskSubtasksSection (task-subtasks-section.tsx:34:38)
    at renderWithHooks (react-dom.development.js:14985:1)
    at updateFunctionComponent (react-dom.development.js:17365:1)
    at beginWork (react-dom.development.js:19072:1)
    at HTMLUnknownElement.callCallback (react-dom.development.js:3945:1)

@XantreDev
Copy link
Contributor Author

@diego9497 can you provide repro? Is it enough to install signals, redux toolkit and use it just once?

@lucasbarrosomk6
Copy link

Ran into this problem as well.
"@preact/signals-react": "1.3.6",
doesn't play well with redux toolkit, is there a solution in the works?

@lucasbarrosomk6
Copy link

It is enough to install signals in a redux toolkit project and use it just once.

@jholmes033169
Copy link

Also getting this issue, also using redux (but not not toolkit). Downgrading did solve the problem, but I don't want to be stuck on 1.2.2. Attempting to upgrade to latest version of redux and see how this affects things.

@XantreDev
Copy link
Contributor Author

Do you need to do something especial in rudux to have this issue. If you will provide it I will try to create test case

@XantreDev
Copy link
Contributor Author

I've tried to reproduce it by writing simple counter, but all is working fine. What do I need to do to reproduce this behaviour?
https://github.com/XantreGodlike/preact-signals-redux-issue

@jholmes033169
Copy link

jholmes033169 commented Nov 13, 2023 via email

@jholmes033169
Copy link

jholmes033169 commented Nov 13, 2023

Ok... so the only thing I need to do to reproduce this is add and setup react-redux in the a project. Straight up react-redux will do it... doesn't even need to be toolkit. I'm using react-redux ^8.0.2, react": "^18.2.0 and "redux": "^4.0.4",

@XantreDev
Copy link
Contributor Author

Seems to be this issue related with react patching. Some default behaviour of react breaks after monkeypatching and this issue appears in big projects

@jholmes033169
Copy link

Yeah... confirmed it with another project with react-redux. I just pulled signals-react in and created a signal and had the same issue. This is a huge problem for migrating. We had hoped to move our thinking gradually from redux to signals. I don't know what the repercussions of sticking with 1.2.2 until we can fully migrate are, though.

@batlash
Copy link

batlash commented Dec 1, 2023

Hi, I had the same problem.
I had the following problem/warning (before I added the signal), which in my case seems to be the root case; Warning: Each child in a list should have a unique "key" prop.
When that warning was resolved, this problem with the signal also was resolved (which caused that the application could not render).
I hope that this could help.

@XantreDev
Copy link
Contributor Author

Warning: Each child in a list should have a unique "key" prop.

So you've had elements with the same key and it caused this error, isn't it?

Cannot read properties of undefined (reading 'length')

@batlash
Copy link

batlash commented Dec 1, 2023

Exactly, (or rather I had no key at all). So the solution in my case was to add a key (in a component that had noting to do with the signal).

(<div className="container">
                    {movies.map((movie) => <MovieCard  key={movie.imdbID} movie={movie} onClick={onCardClicked} />)}
 </div>)


@andrewiggins
Copy link
Member

Our next major version of signals-react (hopefully coming out in the next couple weeks) is gonna deprecate and make opt-in the current React internal patching in favor of using a Babel transform to make components reactive to signals.

The transform is available now on NPM for trying out but some internal changes/fixes & more/updated documentation are needed before I consider it stable. I suspect that the transform won't have similar issues.

@XantreDev
Copy link
Contributor Author

@andrewiggins I don't think this approach scales well - because we will lack support of next.js. Since their babel integration is kinda legacy and not all features will work with babel (next-fonts doesn't). So seems to be we cannot rely on transform, probably there should be transform for other parsers (swc, esbuild).
For now I don't want to increate complexity and prepared manual intergation HOC for such environments, check my integration. I would like you to give me some feedback (it based on transform and signal-react integration source code)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants