solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.42k stars 924 forks source link

Potential breaking changes in 1.7.10 #1848

Closed phungleson closed 1 year ago

phungleson commented 1 year ago

Describe the bug

When I updated to 1.7.10 and ran some tests and I got this error.

TypeError: fn is not a function
 ❯ untrack ../../node_modules/.pnpm/solid-js@1.7.10/node_modules/solid-js/dist/dev.js:486:33
    484| }
    485| function untrack(fn) {
    486|   if (Listener === null) return fn();
       |                                 ^
    487|   const listener = Listener;
    488|   Listener = null;
 ❯ runTop ../../node_modules/.pnpm/solid-js@1.7.10/node_modules/solid-js/dist/dev.js:844:24
 ❯ runUserEffects ../../node_modules/.pnpm/solid-js@1.7.10/node_modules/solid-js/dist/dev.js:973:36
 ❯ ../../node_modules/.pnpm/solid-js@1.7.10/node_modules/solid-js/dist/dev.js:929:34
 ❯ runUpdates ../../node_modules/.pnpm/solid-js@1.7.10/node_modules/solid-js/dist/dev.js:877:17
 ❯ completeUpdates ../../node_modules/.pnpm/solid-js@1.7.10/node_modules/solid-js/dist/dev.js:929:17
 ❯ runUpdates ../../node_modules/.pnpm/solid-js@1.7.10/node_modules/solid-js/dist/dev.js:878:5
 ❯ createRoot ../../node_modules/.pnpm/solid-js@1.7.10/node_modules/solid-js/dist/dev.js:193:12
 ❯ render ../../node_modules/.pnpm/solid-js@1.7.10/node_modules/solid-js/web/dist/dev.js:556:3
 ❯ Module.render ../../node_modules/.pnpm/@solidjs+testing-library@0.8.4_@solidjs+router@0.8.3_solid-js@1.7.10/node_modules/@solidjs/testing-library/dist/index.js:64:65

I checked my code and all untrack is calling with () => {}.

I reverted back to 1.7.9 and tests passed again.

Maybe there are issues with other libraries (i.e. testing library), let me know if you need more information, happy to look into this, thanks!

Your Example Website or App

N/A

Steps to Reproduce the Bug or Issue

N/A

Expected behavior

N/A

Screenshots or Videos

No response

Platform

Additional context

No response

timothyallan commented 1 year ago

I'm getting this exact error as well with the update. The stack trace is massive, and looks like a lot of entries are from Felte: https://felte.dev/docs/solid/getting-started Screenshot 2023-08-09 at 9 16 31 PM

AerisG222 commented 1 year ago

I'm running into the same thing. In my case, I am getting this when I have a component wrapped in <Suspense>. If I remove the Suspense wrapper, then the error goes away. Trying to dig in to see if I can figure out what is going on, but wanted to post in case this helps anyone troubleshoot.

ryansolid commented 1 year ago

Anyone got a quick reproduction? I did change context internals, but it passed local tests, and a streaming hydrating SSR demo I have in the repo. Probably just missed something easy to fix so really helpful if I can get an example. fn not being a function suggests something higher up is not quite right..

EDIT: Actually I think I know what it is.. SuspenseContext has a default value and the old lookup didn't return it so it is improperly thinking nodes are in Suspense. Then later when it tries to run it checks if they are inFallback expecting to run a function and it is just an undefined property on an empty object.