signals icon indicating copy to clipboard operation
signals copied to clipboard

`@preact/signals-react` 1.3.6 is brokes useSyncExternalStoreWithSelector

Open XantreDev opened this issue 2 years ago • 16 comments

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)

XantreDev avatar Sep 10 '23 17:09 XantreDev

I'm having the same issue, I'm using @preact/[email protected], @reduxjs/toolkit"@1.8.5 and [email protected], 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)

diego9497 avatar Sep 13 '23 22:09 diego9497

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

XantreDev avatar Sep 14 '23 10:09 XantreDev

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 avatar Nov 06 '23 19:11 lucasbarrosomk6

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

lucasbarrosomk6 avatar Nov 06 '23 19:11 lucasbarrosomk6

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.

jholmes033169 avatar Nov 10 '23 02:11 jholmes033169

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 avatar Nov 10 '23 21:11 XantreDev

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

XantreDev avatar Nov 12 '23 05:11 XantreDev

Sorry... li will try to set up a test.

On Sun, Nov 12, 2023, 12:47 AM Valerii Smirnov @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/preactjs/signals/issues/411#issuecomment-1807012228, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZDYABBSD2WKLHLH5HAXXTYEBPHPAVCNFSM6AAAAAA4SJNTZKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBXGAYTEMRSHA . You are receiving this because you commented.Message ID: @.***>

jholmes033169 avatar Nov 13 '23 05:11 jholmes033169

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",

jholmes033169 avatar Nov 13 '23 21:11 jholmes033169

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

XantreDev avatar Nov 13 '23 22:11 XantreDev

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.

jholmes033169 avatar Nov 14 '23 02:11 jholmes033169

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.

batlash avatar Dec 01 '23 15:12 batlash

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')

XantreDev avatar Dec 01 '23 15:12 XantreDev

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>)


batlash avatar Dec 01 '23 15:12 batlash

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.

andrewiggins avatar Dec 02 '23 00:12 andrewiggins

@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)

XantreDev avatar Dec 02 '23 14:12 XantreDev