next-view-transitions icon indicating copy to clipboard operation
next-view-transitions copied to clipboard

client-side exception

Open KevinDoughty opened this issue 1 year ago • 9 comments
trafficstars

Steps to reproduce: Click "Go to / demo" and "Open homepage" back and forth a dozen times. Then hit the browser's back navigation button, rapidly, a dozen times. If it doesn’t crash, hit the browser's forward navigation button, rapidly, a dozen times. It usually crashes on me before that.

Using Brave, Ubuntu Screenshot from 2024-04-14 20-41-59

KevinDoughty avatar Apr 17 '24 10:04 KevinDoughty

Ran it in dev mode on my machine Screenshot from 2024-04-17 11-45-39

KevinDoughty avatar Apr 17 '24 11:04 KevinDoughty

I don't think it's possible to disable HMR in Next for a minimal reproduction to get rid of the "Warning: Cannot update a component (HotReload) while rendering a different component (ViewTransitions)."

That stack trace can probably be disregarded though. The async/await trace looks like this:

react-dom.development.js:9157 Uncaught Error: async/await is not yet supported in Client Components, only Server Components. This error is often caused by accidentally adding `'use client'` to a module that was originally written for the server.
    at trackUsedThenable (react-dom.development.js:9157:19)
    at useThenable (react-dom.development.js:11483:16)
    at Object.use (react-dom.development.js:11504:14)
    at use (react.development.js:2572:21)
    at useBrowserNativeTransitions (index.js:47:12)
    at ViewTransitions (index.js:80:5)
    at renderWithHooks (react-dom.development.js:11121:18)
    at updateFunctionComponent (react-dom.development.js:16290:20)
    at beginWork$1 (react-dom.development.js:18472:16)
    at HTMLUnknownElement.callCallback (react-dom.development.js:20565:14)
    at Object.invokeGuardedCallbackImpl (react-dom.development.js:20614:16)
    at invokeGuardedCallback (react-dom.development.js:20689:29)
    at beginWork (react-dom.development.js:26949:7)
    at performUnitOfWork (react-dom.development.js:25748:12)
    at workLoopSync (react-dom.development.js:25464:5)
    at renderRootSync (react-dom.development.js:25419:7)
    at performSyncWorkOnRoot (react-dom.development.js:24887:20)
    at flushSyncWorkAcrossRoots_impl (react-dom.development.js:7758:13)
    at flushSyncWorkOnAllRoots (react-dom.development.js:7718:3)
    at processRootScheduleInMicrotask (react-dom.development.js:7863:3)
    at eval (react-dom.development.js:8034:7)
trackUsedThenable	@	react-dom.development.js:9157
useThenable	@	react-dom.development.js:11483
use	@	react-dom.development.js:11504
use	@	react.development.js:2572
useBrowserNativeTransitions	@	index.js:47
ViewTransitions	@	index.js:80
renderWithHooks	@	react-dom.development.js:11121
updateFunctionComponent	@	react-dom.development.js:16290
beginWork$1	@	react-dom.development.js:18472
callCallback	@	react-dom.development.js:20565
invokeGuardedCallbackImpl	@	react-dom.development.js:20614
invokeGuardedCallback	@	react-dom.development.js:20689
beginWork	@	react-dom.development.js:26949
performUnitOfWork	@	react-dom.development.js:25748
workLoopSync	@	react-dom.development.js:25464
renderRootSync	@	react-dom.development.js:25419
performSyncWorkOnRoot	@	react-dom.development.js:24887
flushSyncWorkAcrossRoots_impl	@	react-dom.development.js:7758
flushSyncWorkOnAllRoots	@	react-dom.development.js:7718
processRootScheduleInMicrotask	@	react-dom.development.js:7863
eval	@	react-dom.development.js:8034

KevinDoughty avatar Apr 17 '24 23:04 KevinDoughty

Same here in Chrome.

Originates at: https://github.com/shuding/next-view-transitions/blob/61d220bc4a1d81697011a7a3e1c4867b4826a329/src/browser-native-events.ts#L65

mpopv avatar Apr 18 '24 19:04 mpopv

I think the issue maybe related to the "use" hook. image React sugguests that we should generate the promise in server component, while here the transition[0] is generated in client side and relayed on browser API. image So the solution maybe passing a context rather than promise to the use hook. I am still trying to fix it :) Metion this in case the duplicated work :)

boombb12138 avatar Apr 20 '24 15:04 boombb12138

Fixed ~ GIF 2024-4-22 11-46-13 I cleaning up the event listener, and everything goes well. image Could you please tell me the issue that may caused by clean up the listener? I don't figure out the potential problems. @shuding

boombb12138 avatar Apr 22 '24 03:04 boombb12138

I can confim https://github.com/shuding/next-view-transitions/pull/8 fixes the client-side exception, but it also completely disables all animation on browser navigation (using the forward and backward button instead of clicking the links).

I think the fault lies with the View-Transitions spec, but will be happy if you can prove me wrong.

KevinDoughty avatar Apr 25 '24 22:04 KevinDoughty

Yes like @KevinDoughty said, I noticed that once removed the event listener in useSyncExternalStore it won't be registered again upon re-rendering. That breaks the browser navigation transitions. And that's why I removed it.

I wasn't sure if it's a React or a Next.js issue and didn't have the time to dig into it.

shuding avatar Apr 26 '24 12:04 shuding

@shuding cc @KevinDoughty @boombb12138 I submitted a PR that fixes this issue & maintain browser navigation transitions. Can you take a look and see if my approach looks good? My approach removes useSyncExternalStore and uses useState + useEffect instead

javascripter avatar May 06 '24 13:05 javascripter

Verified, the https://github.com/shuding/next-view-transitions/pull/11 submitted by @javascripter works on my machine. I cannot cause a client-side exception no matter how hard I try. Transitions happen on browser navigation.

Google dev rel Bramus mentioned on the site formerly known as Bird that they are working on making View Transitions retargetable, which should mean the following visual discrepancies will be fixed. I suspect the API and behavioral impact will be minimal, but will investigate when the time comes.

https://github.com/shuding/next-view-transitions/assets/1807709/be13d685-f130-4ed6-96ba-cf916efd18b8

KevinDoughty avatar May 06 '24 16:05 KevinDoughty