Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeError: this.timerId.unref is not a function - Testing in JSDOM environment #4552

Closed
matt-kinton opened this issue Feb 28, 2021 · 12 comments

Comments

@matt-kinton
Copy link

matt-kinton commented Feb 28, 2021

Describe your environment

  • Operating System version: macOS
  • Browser version: JSDOM
  • Firebase SDK version: 8.2.9
  • Firebase Product: firestore

Describe the problem

Hey guys, I'm trying to test my React application that uses firebase.

For testing I'm running the emulators locally and then using Jest in a JSDOM environment (Create React App setup) to run my tests. All of my auth tests run fine, however when running tests that use firestore I get the the below error:

Error: Uncaught [TypeError: this.timerId.unref is not a function]

I think by the looks of things this is linked to this issue jestjs/jest#1909

As JSDOM emulates the browser I would expect firestore to work. If I use this setInterval().__proto__.unref = function () {};
the test works as expected.

Full stack trace:

    Error: Uncaught [TypeError: this.timerId.unref is not a function]
        at reportException (C:\Users\Matt\Dev\gradeguide-web\node_modules\jsdom\lib\jsdom\living\helpers\runtime-script-errors.js:62:24)
        at Timeout.task [as _onTimeout] (C:\Users\Matt\Dev\gradeguide-web\node_modules\jsdom\lib\jsdom\browser\Window.js:396:9)
        at listOnTimeout (internal/timers.js:554:17)
        at processTimers (internal/timers.js:497:7) TypeError: this.timerId.unref is not a function
        at BackoffTimeout.unref (C:\Users\Matt\Dev\gradeguide-web\node_modules\@grpc\grpc-js\src\backoff-timeout.ts:117:18)
        at new ResolvingLoadBalancer (C:\Users\Matt\Dev\gradeguide-web\node_modules\@grpc\grpc-js\src\resolving-load-balancer.ts:199:25)
        at new ChannelImplementation (C:\Users\Matt\Dev\gradeguide-web\node_modules\@grpc\grpc-js\src\channel.ts:242:34)
        at new Client (C:\Users\Matt\Dev\gradeguide-web\node_modules\@grpc\grpc-js\src\client.ts:146:30)
        at new ServiceClientImpl (C:\Users\Matt\Dev\gradeguide-web\node_modules\@grpc\grpc-js\src\make-client.ts:128:3)
        at GrpcConnection.Object.<anonymous>.GrpcConnection.ensureActiveStub (C:\Users\Matt\Dev\gradeguide-web\node_modules\@firebase\firestore\src\platform\node\grpc_connection.ts:90:25)
        at GrpcConnection.Object.<anonymous>.GrpcConnection.openStream (C:\Users\Matt\Dev\gradeguide-web\node_modules\@firebase\firestore\src\platform\node\grpc_connection.ts:175:23) 
        at PersistentWriteStream.Object.<anonymous>.PersistentWriteStream.startRpc (C:\Users\Matt\Dev\gradeguide-web\node_modules\@firebase\firestore\src\remote\persistent_stream.ts:710:28)
        at PersistentWriteStream.Object.<anonymous>.PersistentStream.startStream (C:\Users\Matt\Dev\gradeguide-web\node_modules\@firebase\firestore\src\remote\persistent_stream.ts:443:24)
        at call (C:\Users\Matt\Dev\gradeguide-web\node_modules\@firebase\firestore\src\remote\persistent_stream.ts:420:16)
        at call (C:\Users\Matt\Dev\gradeguide-web\node_modules\@firebase\node_modules\google-closure-library\closure\goog\promise\promise.js:826:22)
        at goog.Promise.invokeCallback_ (C:\Users\Matt\Dev\gradeguide-web\node_modules\@firebase\node_modules\google-closure-library\closure\goog\promise\promise.js:1166:5)
        at executeCallback_ (C:\Users\Matt\Dev\gradeguide-web\node_modules\@firebase\node_modules\google-closure-library\closure\goog\promise\promise.js:1140:5)
        at D.call [as gc] (C:\Users\Matt\Dev\gradeguide-web\node_modules\@firebase\node_modules\google-closure-library\closure\goog\promise\promise.js:1111:5)
        at xc (C:\Users\Matt\Dev\gradeguide-web\node_modules\@firebase\node_modules\google-closure-library\closure\goog\async\run.js:124:7)
        at processTicksAndRejections (internal/process/task_queues.js:93:5)

      at VirtualConsole.<anonymous> (node_modules/jsdom/lib/jsdom/virtual-console.js:29:45)
      at reportException (node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:66:28)
      at Timeout.task [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:396:9)```
@matt-kinton
Copy link
Author

matt-kinton commented Feb 28, 2021

Update - After messing around for a while, using the setInterval patch above, allows the tests to run but gives me a 'Jest did not exist one second after the test run has completed' warning messaged. Using jest.useFakeTimers(); removes that error but doesnt actually allow the firestore request to complete.

Here are some issues that look like it's related to:
jsdom/jsdom#2617
grpc/grpc-node#1077
facebook/create-react-app#2590

@hsubox76
Copy link
Contributor

hsubox76 commented Mar 1, 2021

Related to #4557 but seems like that user isn't using Jest.

Seems like you may be getting the Node bundle of Firestore when running Jest tests, if everything in Jest should be using browser APIs is there a webpack config or other setting that you can change to point to the browser field of package.json instead of main? Then I believe it won't use grpc, and it won't try to use that native Node method.

@matt-kinton
Copy link
Author

@hsubox76 thanks for looking into this. From what I can see CRA extracts any setting like you suggest that would resolve the problem so I have raised an issue there.

@matt-kinton
Copy link
Author

matt-kinton commented Mar 2, 2021

I've been trying to get this fix to work facebook/create-react-app#2590 (comment) but with no luck. I can see that firebase-browser.js doesn't exist anymore but is there a new equivalent?

Update: Using this

jest.mock("firebase/app", () => require("firebase/firebase-app.js"));
jest.mock("firebase/firestore", () =>
  require("firebase/firebase-firestore.js")
);
jest.mock("firebase/auth", () => require("firebase/firebase-auth.js"));

I get this error

Warning: This is a browser-targeted Firebase bundle but it appears it is being
         run in a Node environment.  If running in a Node environment, make sure you
         are using the bundle specified by the "main" field in package.json.
         
         If you are using Webpack, you can specify "main" as the first item in
         "resolve.mainFields":
         https://webpack.js.org/configuration/resolve/#resolvemainfields
         
         If using Rollup, use the @rollup/plugin-node-resolve plugin and specify "main"
         as the first item in "mainFields", e.g. ['main', 'module'].
         https://github.com/rollup/@rollup/plugin-node-resolve

Update update: OK, after some more playing around, isNode used here:

if (isNode()) {
)

Is true in a JSDOM environment, whereas it is false in an actual browser (obviously).

The isNode/isBrowser logic I think needs updating. See here https://github.com/flexdinesh/browser-or-node/blob/master/src/index.js might be some good inspiration, which also provides an extra flag for isJsDom

@hsubox76
Copy link
Contributor

hsubox76 commented Mar 3, 2021

Is that an error? I think it's just a warning. Nothing happens in that if (isNode()) block except a logger.warn() statement. But if it's an incorrect warning we should fix it, I'll look into it.

The firebase/firebase-*.js files probably aren't the ones you want. Those are bundles for the CDN and are going to be a bit less optimized, although maybe it doesn't matter for tests. If you want to test the actual files your React application is probably using, you probably should be looking at packages/[package name]/dist/index.esm.js files. If you definitely can't find any setting to prefer the browser field, this is basically you doing it manually.

Setting webpack (or any bundler) to prefer the browser field is basically the same as you manually going into every firebase/[package] directory and looking at its package.json, finding the file pointed to in the browser field, and manually forcing the import of that specific file. So for example, for firebase/app, you would go into /node_modules/firebase/app (NOT node_modules/@firebase/app) and look at the package.json found there, which has entries for main, browser, and module. The entry for browser is "browser": "dist/index.esm.js" so that means that if you can force your application (your jest test I suppose) to get a specific file whenever firebase/app is imported (which I assume is what jest.mock is doing) then the path would be firebase/app/dist/index.esm.js.

So that's basically the exact same thing webpack would be doing automatically for you if could configure it. So if you can't I guess that's a solution. Doesn't seem ideal to not work "out of the box" with such a popular tool but I can't think of what we could do on our end. If anyone has any ideas, let me know.

@hsubox76
Copy link
Contributor

hsubox76 commented Mar 3, 2021

Reading the (old) linked issue in the CRA repo it makes sense they wouldn't want to force a default to the browser field, but I think maybe the best solution would be for CRA to provide some kind of easy option for the user to change that config without ejecting.

@matt-kinton
Copy link
Author

matt-kinton commented Mar 3, 2021

Unfortunately using

jest.mock("firebase/app", () => require("firebase/app/dist/index.esm.js"));
jest.mock("firebase/firestore", () =>
  require("firebase/firestore/dist/index.esm.js")
);
jest.mock("firebase/auth", () => require("firebase/auth/dist/index.esm.js"));

throws this error SyntaxError: Cannot use import statement outside a module. Using the firebase/firebase-*.js files doesn't throw that error which is why I was using those. They do however lead to that warning above, which as you mention is only a warning but I know that isNode() is used throughout the repository so I'm wondering if its use else where is causing the problem as it will be returning true, even though it should really be false for JSDOM. The actual error I get when using the firebase/firebase-*.js files is TypeError: app.firestore is not a function so I'm not sure what is causing that.

If I use jest.mock("firebase", () => require("firebase/firebase.js")); which I was hoping would replace everything I get ReferenceError: IDBIndex is not defined which having a google I can see you've encountered before.

What's also confusing is that ReacfFire's tests use jsdom by the looks of things and aren't doing anything out of the ordinary.

https://github.com/FirebaseExtended/reactfire

@maierson
Copy link

I am also running into this. The only way I am able to do e2e tests on a component that uses Firebase emulator is via Cypress (in other words in a real browser). @Mattinton any progress?

@matt-kinton
Copy link
Author

Unfortunately not @maierson. I began yesterday experimenting with a mocking system for firebase and ditched the emulators/using real firebase completely. Hoping that in the future I'll be able to revert back to the emulators but for now mocking is just far more reliable, even if my tests become more unit testy than integration testy.

@nicograef
Copy link

nicograef commented Mar 11, 2021

Hey @maierson @Mattinton @hsubox76

this is a error in grpc-js!

It was introduced in grpc-js v1.2.7 here: grpc/grpc-node#1688

And it is fixed with this PR: grpc/grpc-node#1709

Please include the fixed version (as soon as released) into the next firebase release :)

@schmidt-sebastian
Copy link
Contributor

We auto-update our dependencies as soon as the fix in @grpc/grpc-js has been released. Thank you for reporting this and following up. Let's continue this discussion in grpc/grpc-node#1708

@nicograef
Copy link

Thank you! :)

@firebase firebase locked and limited conversation to collaborators Apr 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants