loki icon indicating copy to clipboard operation
loki copied to clipboard

runtime-error's on certain VR tests.

Open daveholst opened this issue 2 years ago • 3 comments

We were running into some errors when trying to run Loki on a new component/story that we had just made. The component works 100% fine in storybook.

This was the error we were getting:

 PASS  chrome.docker/chrome.a4/Forms/TextInput
 PASS  chrome.docker/chrome.a4/Components/IconButton
 PASS  chrome.docker/chrome.a4/Components/Modals/DjangoModal
 PASS  chrome.docker/chrome.a4/Components/Modals/DjangoPopUpModal
 FAIL  chrome.docker/chrome.a4/Components/Modals/Modal
       Default
       1 request failed to load; http://192.168.1.236:6006/runtime-error
       Default Open
       1 request failed to load; http://192.168.1.236:6006/runtime-error
       Form Elements
       1 request failed to load; http://192.168.1.236:6006/runtime-error
 PASS  chrome.docker/chrome.a4/Typography/Heading

After digging around in the chrome container logs I worked out that it was related to this:

2023-01-06 11:27:39 [0106/032739.394793:INFO:CONSOLE(95144)] "Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?%s%s 
2023-01-06 11:27:39 
2023-01-06 11:27:39 Check the render method of `SlotClone`. 
2023-01-06 11:27:39     in Heading (created by SlotClone)
2023-01-06 11:27:39     in SlotClone (created by Slot)
2023-01-06 11:27:39     in Slot (created by Primitive.h2)
2023-01-06 11:27:39     in Primitive.h2 (created by DialogTitle)
2023-01-06 11:27:39     in DialogTitle (created by Modal)
2023-01-06 11:27:39     in div (created by Modal)
2023-01-06 11:27:39     in div (created by Modal)
2023-01-06 11:27:39     in div (created by Modal)
2023-01-06 11:27:39     in div (created by Primitive.div)
2023-01-06 11:27:39     in Primitive.div (created by DismissableLayer)
2023-01-06 11:27:39     in DismissableLayer (created by SlotClone)
2023-01-06 11:27:39     in SlotClone (created by Slot)
2023-01-06 11:27:39     in Slot (created by Primitive.div)
2023-01-06 11:27:39     in Primitive.div (created by FocusScope)
2023-01-06 11:27:39     in FocusScope (created by ForwardRef)
2023-01-06 11:27:39     in ForwardRef (created by ForwardRef)
2023-01-06 11:27:39     in ForwardRef (created by Presence)
2023-01-06 11:27:39     in Presence (created by DialogContent)
2023-01-06 11:27:39     in DialogContent (created by SlotClone)
2023-01-06 11:27:39     in SlotClone (created by Slot)
2023-01-06 11:27:39     in Slot (created by Primitive.div)
2023-01-06 11:27:39     in Primitive.div (created by Portal)
2023-01-06 11:27:39     in Portal (created by Presence)
2023-01-06 11:27:39     in Presence (created by DialogPortal)
2023-01-06 11:27:39     in DialogPortalProvider (created by DialogPortal)
2023-01-06 11:27:39     in DialogPortal (created by Modal)
2023-01-06 11:27:39     in DialogProvider (created by Dialog)
2023-01-06 11:27:39     in Dialog (created by Modal)
2023-01-06 11:27:39     in Modal (created by unboundStoryFn)
2023-01-06 11:27:39     in unboundStoryFn
2023-01-06 11:27:39     in ErrorBoundary", source: http://192.168.10.16:6006/vendors-node_modules_babel_runtime-corejs2_core-js_object_assign_js-node_modules_babel_runtim-56dea2.iframe.bundle.js (95144)
2023-01-06 11:27:39 [0106/032739.515549:INFO:CONSOLE(32)] "Uncaught TypeError: resolveRAF is not a function", source:  (32)

I believe a colleague also had seen this error a few weeks ago when they had some forwardRef logic in a react component, but he adjusted the way he was handling the types. We unhandily both couldn't really remember how we solved it 😓 . Our components and stories are all written in TS.

I had a snoop around for resolveRAF to see if I could see what was going on. I think in node_modules/@loki/browser/src/disable-animations.js it was getting to a state where resolveRAF was trying to be invoked, but not a defined function on line 34.

If I manually change the above file @ line 34 to do a truthy check first like this:

        if(resolveRAF) resolveRAF();

All our VR tests work with the expected outputs.

I also noticed that if I push the timeout from 50 -> 250 on line 38 my VR's also work with no errors.

    setTimeout(() => {
      currentFrame++;
      callbacks.splice(0).forEach((c) => c(now()));

      // Assume no new invocations for 50ms means we've ended
      resolveRAFTimer = setTimeout(() => {
        // if (resolveRAF) resolveRAF();
        resolveRAF();
        resolveRAF = null;
        resolveRAFTimer = null;
      }, 250); // manually changed
    }, 0);

Keen for advice on how to proceed with this one!

daveholst avatar Jan 07 '23 01:01 daveholst

Sounds like you've found something here! Would you mind creating a minimal reproducible example that I can play around with and add to the test suite? I do already have some using RAF, but it seems you're doing something slightly different perhaps.

Have a look here for inspiration: https://github.com/oblador/loki/blob/master/examples/react/src/async/Animation.stories.js

oblador avatar Jan 09 '23 10:01 oblador

@oblador Been a busy week, but I managed to have a look into this on Friday. After forking the project and trying to creat a slim example of our failure (has taken a bit as I have to pull out the tailwind and typescript) I can't get it to error using the fork, but will still keep trying.

I did notice that the Animation tests were failing with the same error I was getting. Would you be able to see if you are also getting this your end? Hopefully it is failing in the same way I am experiencing and can help work out what is breaking🤞 image

Thanks for the help on this one!

daveholst avatar Jan 16 '23 01:01 daveholst

Having the same issue here. We removed framer-motion from our stack because of it, but now it is also failing on our RTE component using tip-tap :pensive: Increasing the timeout to 5s allowed us to generate screenshots, but this is not exactly a long term solution. A short term solution could be to allow users to setup a custom timeout in the config?

Edit: We ended up using patch-package to force patch Loki and have a longer timeout

Euregan avatar Sep 15 '23 12:09 Euregan