jeremyckahn / shifty

The fastest TypeScript animation engine on the web
https://jeremyckahn.github.io/shifty/doc/
MIT License
1.54k stars 87 forks source link

Jest detecting open handle from shifty require #156

Closed BoldBigflank closed 2 years ago

BoldBigflank commented 2 years ago

When I load shifty like so

const { interpolate } = require('shifty')

My jest tests that load this module will not exit properly. When I run

 npx jest --detectOpenHandles --bail

The result suggests that the require is a "open handle potentially keeping Jest from exiting".

I'm kind of inexperienced in this, but does this make sense even though there isn't really any Promise/async stuff happening in the require?

jeremyckahn commented 2 years ago

Hi @BoldBigflank, thanks for reporting! This isn't an issue I've seen before. Based on the error you're seeing, I would assume that it's related to a tween's Promise not being resolved appropriately, but you mentioned that you're not doing any asynchronous work so that wouldn't make sense.

What happens when you use ES module syntax? So, try converting from:

const { interpolate } = require('shifty')

To:

import { interpolate } from 'shifty'

That should be equivalent, but perhaps there is some discrepancy in the implementation details related to require in your environment. It could even be that the native Node require got overwritten with a non-native implementation that returns a Promise.

Would it be possible to share the project you're experiencing this in? It's hard to tell what's causing this based in the information that's been provided so far.

BoldBigflank commented 2 years ago

I still have the issue with shifty when I have the most bare bones example:

const { interpolate } = require('shifty')

describe('Barebones example', () => {
    test('sum', () => {
        const sum = 1 + 1
        expect(sum).toEqual(2)
    })
})

Here's the stack trace:

Ran all test suites matching /test\/barebones.test.js/i.

Jest has detected the following 1 open handle potentially keeping Jest from exiting:

  ●  Timeout

    > 1 | const { interpolate } = require('shifty')
        |                         ^
      2 |
      3 | describe('Barebones example', () => {
      4 |     test('sum', () => {

      at scheduleUpdate (node_modules/shifty/dist/webpack:/shifty/src/tweenable.js:214:20)
      at Object.720 (node_modules/shifty/dist/webpack:/shifty/src/tweenable.js:824:1)
      at __webpack_require__ (node_modules/shifty/dist/webpack:/shifty/webpack/bootstrap:18:1)
      at node_modules/shifty/dist/webpack:/shifty/webpack/startup:4:1
      at node_modules/shifty/dist/shifty.node.js:2569:12
      at webpackUniversalModuleDefinition (node_modules/shifty/dist/webpack:/shifty/webpack/universalModuleDefinition:3:1)
      at Object.<anonymous> (node_modules/shifty/dist/webpack:/shifty/webpack/universalModuleDefinition:10:2)
      at Object.<anonymous> (test/barebones.test.js:1:25)
      at TestScheduler.scheduleTests (node_modules/@jest/core/build/TestScheduler.js:333:13)
      at runJest (node_modules/@jest/core/build/runJest.js:404:19)
      at _run10000 (node_modules/@jest/core/build/cli/index.js:320:7)
      at runCLI (node_modules/@jest/core/build/cli/index.js:173:3)

I suppose since the test is so simple, it might be something in the Jest config...

jeremyckahn commented 2 years ago

Thanks for this additional information! I am seeing this as well, but only on Jest version 28. On version 27 and below, I do not see the error. I wonder if this is a Jest bug? In any case, I'll do some more exploration to see if there's something that can be changed within Shifty to prevent this issue.

jeremyckahn commented 2 years ago

I'm experiencing this issue within Shifty's own tests when I upgrade to Jest 28. I'll explore a fix!

BoldBigflank commented 2 years ago

When I checked I was using Jest v27.5.1, and for some reason attempting to get to 27.0.1 always installs 27.5.1. Thanks for checking this out, I'm glad it's not just me with this issue.

jeremyckahn commented 2 years ago

For sure! Thanks again for reporting. Just to share, I upgraded Jest to version 28. When I run CI=true npx jest --detectOpenHandles, I get:

$: CI=true npx jest --detectOpenHandles | pbcopy
npm WARN config tmp This setting is no longer used.  npm stores temporary files in a special
npm WARN config location in the cache, and they are managed by
npm WARN config     [`cacache`](http://npm.im/cacache).
PASS src/tweenable.test.js
PASS src/token.test.js
PASS src/scene.test.js
PASS src/bezier.test.js
PASS src/interpolate.test.js

Test Suites: 5 passed, 5 total
Tests:       70 passed, 70 total
Snapshots:   0 total
Time:        0.792 s, estimated 1 s
Ran all test suites.

Jest has detected the following 4 open handles potentially keeping Jest from exiting:

  ●  Timeout

      212 | export const scheduleUpdate = () => {
      213 |   now = getCurrentTime()
    > 214 |   scheduleFunction.call(root, scheduleUpdate, UPDATE_TIME)
          |                    ^
      215 |
      216 |   processTweens()
      217 | }

      at call (tweenable.js:214:20)
      at Object.scheduleUpdate (tweenable.js:822:1)
      at Object.<anonymous> (index.js:5:1)
      at Object.<anonymous> (interpolate.test.js:1:1)

  ●  Timeout

      212 | export const scheduleUpdate = () => {
      213 |   now = getCurrentTime()
    > 214 |   scheduleFunction.call(root, scheduleUpdate, UPDATE_TIME)
          |                    ^
      215 |
      216 |   processTweens()
      217 | }

      at call (tweenable.js:214:20)
      at Object.scheduleUpdate (tweenable.js:822:1)
      at Object.<anonymous> (index.js:5:1)
      at Object.<anonymous> (bezier.test.js:1:1)

  ●  Timeout

      212 | export const scheduleUpdate = () => {
      213 |   now = getCurrentTime()
    > 214 |   scheduleFunction.call(root, scheduleUpdate, UPDATE_TIME)
          |                    ^
      215 |
      216 |   processTweens()
      217 | }

      at call (tweenable.js:214:20)
      at Object.scheduleUpdate (tweenable.js:822:1)
      at Object.<anonymous> (scene.test.js:1:1)

  ●  Timeout

      212 | export const scheduleUpdate = () => {
      213 |   now = getCurrentTime()
    > 214 |   scheduleFunction.call(root, scheduleUpdate, UPDATE_TIME)
          |                    ^
      215 |
      216 |   processTweens()
      217 | }

      at call (tweenable.js:214:20)
      at Object.scheduleUpdate (tweenable.js:822:1)
      at Object.<anonymous> (index.js:5:1)
      at Object.<anonymous> (token.test.js:1:1)

So I think there's something that can change within Shifty to address this. I'm still digging!

jeremyckahn commented 2 years ago

I think this is because Shifty uses a continuous heartbeat to run pending animations: https://github.com/jeremyckahn/shifty/blob/e4d6b404ece5161cf057f8b841e9a79936dfe1a7/src/tweenable.js#L212-L217

It gets kicked off here: https://github.com/jeremyckahn/shifty/blob/e4d6b404ece5161cf057f8b841e9a79936dfe1a7/src/tweenable.js#L822

This is core to Shifty's design, but it runs afoul of what Jest is expecting to happen. I think we'll need some conditional logic to prevent scheduling of a heartbeat tick if there are no pending animations. :thinking:

jeremyckahn commented 2 years ago

@BoldBigflank I've got a bugfix in the works at #157. I also published a pre-release version of Shifty (2.19.0-0). Would you mind installing that version (npm i --save 2.19.0-0) and seeing if adding something like the following to your Jest setup file addresses the issue for you?

import { shouldScheduleUpdate } from 'shifty'

afterAll(() => {
  shouldScheduleUpdate(false)
})
BoldBigflank commented 2 years ago

I tried out 2.19.0-0 and added the afterAll to the barebones test, and that completes with no errors, which is great. With this setup, I'd likely just add shouldScheduleUpdate(false) to the modules that import interpolate since that's the only function we're using (creating gifs on the back end, not real time, so syncing isn't important).

jeremyckahn commented 2 years ago

Awesome, thank you for checking @BoldBigflank. A more narrowly-scoped use of shouldScheduleUpdate(false) should work just fine. I will release the fix in 2.19.0!