fix: React next fails on build (#726) #732

MatanBobi · 2020-07-05T06:00:26Z

What: I've added a scheduler-compat module which will mock unstable_scheduleCallback for version of react that don't have the Scheduler package.

Why: Following #726, our build was failing for react@next so we needed to change the way we flush micro tasks.

How: I took act-compat as an example and took @kentcdodds code example here.
The only thing I added was an error handler because I had to suppress the error.

Checklist:

Documentation added to the docs site - Let me know if any documentation for this one is needed.
Tests - I'm working on tests for this, it's failing in CI for Coverage.
Typescript definitions updated - N/A
Ready to be merged

This is a Draft PR just to get initial feedback, the code isn't good enough yet.

codesandbox-ci · 2020-07-05T06:06:55Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 5d9f64d:

Sandbox	Source
new-wildflower-zlfoo	Configuration
dank-cache-7m81x	Configuration

kentcdodds

Thanks for this! What do you think of my suggestions?

src/flush-microtasks.js

kentcdodds · 2020-07-05T13:54:19Z

src/flush-microtasks.js

+        scheduleCallback(null, () => {
+          enqueueTask(() => {
+            resolve()
+          })
+        })


With the change above, we no longer need this first argument

Suggested change

scheduleCallback(null, () => {

enqueueTask(() => {

resolve()

})

})

scheduleCallback(() => enqueueTask(resolve))

kentcdodds · 2020-07-05T13:55:00Z

src/scheduler-compat.js

+const globalObj = typeof window === 'undefined' ? global : window
+
+let Scheduler = globalObj.Scheduler
+try {
+  const requireString = `require${Math.random()}`.slice(0, 7)
+  const nodeRequire = module && module[requireString]
+  // import React's scheduler so we'll be able to schedule our tasks later on.
+  Scheduler = nodeRequire.call(module, 'scheduler')
+} catch (_err) {
+  console.error("The react version you're using doesn't support Scheduling")
+}
+// in case this react version has a Scheduler implementation, we use it,
+// if not, we just create a function calling our callback
+const NormalPriority = Scheduler
+  ? Scheduler.NormalPriority || Scheduler.unstable_NormalPriority
+  : null
+
+const isScheduleCallbackSupported = Scheduler !== undefined
+let isModernScheduleCallbackSupported = null
+
+const errorHandler = e => {
+  // If the error originated from Scheduler, it means we're in v16.8.6
+  if (
+    e.message === 'callback is not a function' &&
+    e.filename.includes('scheduler')
+  ) {
+    console.error(e.error.stack, e.error.detail)
+    e.preventDefault()
+  } else {
+    console.error(e.error)
+  }
+}
+
+export default function scheduleCallback(_, cb) {
+  if (!isScheduleCallbackSupported) {
+    return cb()
+  }
+
+  if (isModernScheduleCallbackSupported === null) {
+    // patch console.error here
+    const originalConsoleError = console.error
+    console.error = function error(...args) {
+      /* if console.error fired *with that specific message* */
+      /* istanbul ignore next */
+      const firstArgIsString = typeof args[0] === 'string'
+      if (
+        firstArgIsString &&
+        args[0].indexOf('TypeError: callback is not a function') === 0
+      ) {
+        // v16.8.6
+        isModernScheduleCallbackSupported = false
+        globalObj.removeEventListener('error', errorHandler)
+        console.error = originalConsoleError
+        return cb()
+      } else {
+        originalConsoleError.apply(console, args)
+        console.error = originalConsoleError
+        return cb()
+      }
+    }
+
+    globalObj.addEventListener('error', errorHandler)
+    return Scheduler.unstable_scheduleCallback(NormalPriority, () => {
+      console.error = originalConsoleError
+      isModernScheduleCallbackSupported = true
+      globalObj.removeEventListener('error', errorHandler)
+      return cb()
+    })
+  } else if (isModernScheduleCallbackSupported === false) {
+    return cb()
+  }
+
+  return Scheduler.unstable_scheduleCallback(NormalPriority, cb)
+}
+
+/* eslint no-console:0 */


We don't need this file anymore.

Suggested change

const globalObj = typeof window === 'undefined' ? global : window

let Scheduler = globalObj.Scheduler

try {

const requireString = `require${Math.random()}`.slice(0, 7)

const nodeRequire = module && module[requireString]

// import React's scheduler so we'll be able to schedule our tasks later on.

Scheduler = nodeRequire.call(module, 'scheduler')

} catch (_err) {

console.error("The react version you're using doesn't support Scheduling")

}

// in case this react version has a Scheduler implementation, we use it,

// if not, we just create a function calling our callback

const NormalPriority = Scheduler

? Scheduler.NormalPriority || Scheduler.unstable_NormalPriority

: null

const isScheduleCallbackSupported = Scheduler !== undefined

let isModernScheduleCallbackSupported = null

const errorHandler = e => {

// If the error originated from Scheduler, it means we're in v16.8.6

if (

e.message === 'callback is not a function' &&

e.filename.includes('scheduler')

) {

console.error(e.error.stack, e.error.detail)

e.preventDefault()

} else {

console.error(e.error)

}

}

export default function scheduleCallback(_, cb) {

if (!isScheduleCallbackSupported) {

return cb()

}

if (isModernScheduleCallbackSupported === null) {

// patch console.error here

const originalConsoleError = console.error

console.error = function error(...args) {

/* if console.error fired *with that specific message* */

/* istanbul ignore next */

const firstArgIsString = typeof args[0] === 'string'

if (

firstArgIsString &&

args[0].indexOf('TypeError: callback is not a function') === 0

) {

// v16.8.6

isModernScheduleCallbackSupported = false

globalObj.removeEventListener('error', errorHandler)

console.error = originalConsoleError

return cb()

} else {

originalConsoleError.apply(console, args)

console.error = originalConsoleError

return cb()

}

}

globalObj.addEventListener('error', errorHandler)

return Scheduler.unstable_scheduleCallback(NormalPriority, () => {

console.error = originalConsoleError

isModernScheduleCallbackSupported = true

globalObj.removeEventListener('error', errorHandler)

return cb()

})

} else if (isModernScheduleCallbackSupported === false) {

return cb()

}

return Scheduler.unstable_scheduleCallback(NormalPriority, cb)

}

/* eslint no-console:0 */

kentcdodds · 2020-07-05T13:55:07Z

src/flush-microtasks.js

@@ -1,3 +1,5 @@
+import scheduleCallback from './scheduler-compat'


I don't think having the compat file is necessary.

Suggested change

import scheduleCallback from './scheduler-compat'

import React from 'react'

import semver from 'semver'

const globalObj = typeof window === 'undefined' ? global : window

let Scheduler = globalObj.Scheduler

const isModernScheduleCallbackSupported = semver.satisfies(React.version, '>=16.8.6')

Also, let's move this code to below the comments, just above the getIsUsingFakeTimers

You're definitely right about this one, I thought we don't want to use semver.. This was my go-to solution but since act-compat isn't using it and I didn't see it in the project, I didn't want to add it.
This simplifies everything.

MatanBobi · 2020-07-05T14:38:50Z

Thanks for this! What do you think of my suggestions?

I'm on it, sorry for making this PR too exhaustive, didn't think we'll be good with using semver :)
Thanks for the comments!

kentcdodds

LGTM!

kentcdodds · 2020-07-05T17:16:43Z

@all-contributors please add @MatanBobi for code

allcontributors · 2020-07-05T17:16:54Z

@kentcdodds

I've put up a pull request to add @MatanBobi! 🎉

kentcdodds · 2020-07-05T17:20:09Z

Thanks so much for your help! I've added you as a collaborator on the project (with write access now). Please make sure that you review the other/MAINTAINING.md and CONTRIBUTING.md files (specifically the bit about the commit messages and the git hooks) and familiarize yourself with the code of conduct (we're using the contributor covenant). You might also want to watch the repo to be notified when someone files an issue/PR. Please continue to make PRs as you feel the need (you can make your branches directly on the repo rather than your fork if you want). Thanks! And welcome to the team :)

MatanBobi · 2020-07-05T17:25:27Z

Thanks a lot @kentcdodds!

kentcdodds · 2020-07-05T17:29:27Z

🎉 This PR is included in version 10.4.4 🎉

The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot 📦🚀

MatanBobi added 3 commits July 4, 2020 14:34

fix: Fix build on react@next

f9557bf

Working on ^16.9 and lower than 16.8

5ef6cb8

Solution works in 16.8 also

5d9f64d

kentcdodds reviewed Jul 5, 2020

View reviewed changes

MatanBobi added 2 commits July 5, 2020 17:53

Fixes from code review

ee484fe

Import only the function we need

081cccb

kentcdodds approved these changes Jul 5, 2020

View reviewed changes

kentcdodds marked this pull request as ready for review July 5, 2020 17:14

kentcdodds merged commit 604d3e9 into testing-library:master Jul 5, 2020

allcontributors bot mentioned this pull request Jul 5, 2020

docs: add MatanBobi as a contributor #733

Merged

kentcdodds added the released label Jul 5, 2020

awesomeunleashed mentioned this pull request Jul 14, 2020

10.4.4 causing timeout issue with useEffect+jest.useFakeTimers testing-library/testing-library-docs#523

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: React next fails on build (#726) #732

fix: React next fails on build (#726) #732

MatanBobi commented Jul 5, 2020 •

edited

codesandbox-ci bot commented Jul 5, 2020

kentcdodds left a comment

kentcdodds Jul 5, 2020

kentcdodds Jul 5, 2020

kentcdodds Jul 5, 2020

MatanBobi Jul 5, 2020

MatanBobi commented Jul 5, 2020 •

edited

kentcdodds left a comment

kentcdodds commented Jul 5, 2020

allcontributors bot commented Jul 5, 2020

kentcdodds commented Jul 5, 2020

MatanBobi commented Jul 5, 2020

kentcdodds commented Jul 5, 2020

		@@ -1,3 +1,5 @@
		import scheduleCallback from './scheduler-compat'

fix: React next fails on build (#726) #732

fix: React next fails on build (#726) #732

Conversation

MatanBobi commented Jul 5, 2020 • edited

codesandbox-ci bot commented Jul 5, 2020

kentcdodds left a comment

Choose a reason for hiding this comment

kentcdodds Jul 5, 2020

Choose a reason for hiding this comment

kentcdodds Jul 5, 2020

Choose a reason for hiding this comment

kentcdodds Jul 5, 2020

Choose a reason for hiding this comment

MatanBobi Jul 5, 2020

Choose a reason for hiding this comment

MatanBobi commented Jul 5, 2020 • edited

kentcdodds left a comment

Choose a reason for hiding this comment

kentcdodds commented Jul 5, 2020

allcontributors bot commented Jul 5, 2020

kentcdodds commented Jul 5, 2020

MatanBobi commented Jul 5, 2020

kentcdodds commented Jul 5, 2020

MatanBobi commented Jul 5, 2020 •

edited

MatanBobi commented Jul 5, 2020 •

edited