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: performance.markResourceTiming is not a function #1851

Closed
4 tasks done
robdodson opened this issue Nov 11, 2023 · 18 comments
Closed
4 tasks done

TypeError: performance.markResourceTiming is not a function #1851

robdodson opened this issue Nov 11, 2023 · 18 comments
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node

Comments

@robdodson
Copy link

robdodson commented Nov 11, 2023

Prerequisites

Environment check

  • I'm using the latest msw version
  • I'm using Node.js version 18 or higher

Node.js version

20.9.0

Reproduction steps

Current behavior

undici will throw an error: TypeError: performance.markResourceTiming is not a function. I'm guessing this is because performance.markResourceTiming is not a browser API, but the jest.polyfills.js file replaces fetch with undici which attempts to call this Node API here.

Expected behavior

fetch should work

I worked around the issue by adding performance to my jest.polyfills.js.

/* global globalThis */

/**
 * @note The block below contains polyfills for Node.js globals
 * required for Jest to function when running JSDOM tests.
 * These HAVE to be require's and HAVE to be in this exact
 * order, since "undici" depends on the "TextEncoder" global API.
 *
 * Consider migrating to a more modern test runner if
 * you don't want to deal with this.
 */

const { performance } = require('node:perf_hooks')
const { TextDecoder, TextEncoder } = require('node:util')

Object.defineProperties(globalThis, {
  TextDecoder: { value: TextDecoder },
  TextEncoder: { value: TextEncoder },
  performance: { value: performance },
})

const { Blob } = require('node:buffer')
const { fetch, Headers, FormData, Request, Response } = require('undici')

Object.defineProperties(globalThis, {
  fetch: { value: fetch, writable: true },
  Blob: { value: Blob },
  Headers: { value: Headers },
  FormData: { value: FormData },
  Request: { value: Request },
  Response: { value: Response },
})

Apologies for not creating a reproduction repository. Hopefully this is the correct workaround and it helps someone.

@robdodson robdodson added bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node labels Nov 11, 2023
@kettanaito
Copy link
Member

Hi, @robdodson. This error implies you are running a version of Node.js older than 18.2.0 (see this). Please upgrade your Node.js version and that will fix the issue.

@robdodson
Copy link
Author

It's possible that something is wrong with my setup, but logging process.version during the test setup gives me v20.9.0

console.log
    process.version v20.9.0

      at Object.log (admin/setupTests.js:11:9)

  console.warn
    [MSW] Warning: intercepted a request without a matching request handler:
    
      • GET /profile/user
    
    If you still wish to intercept this unhandled request, please create a request handler for it.
    Read more: https://mswjs.io/docs/getting-started/mocks

      at Object.warn (node_modules/msw/lib/core/utils/internal/devUtils.js:31:11)
      at applyStrategy (node_modules/msw/lib/core/utils/request/onUnhandledRequest.js:170:36)
      at node_modules/msw/lib/core/utils/request/onUnhandledRequest.js:191:5
      at fulfilled (node_modules/msw/lib/core/utils/request/onUnhandledRequest.js:33:24)

  console.error
    Error: Uncaught [TypeError: performance.markResourceTiming is not a function]

I'll see if I can put together a minimal reproduction at some point. It's a little tough because our app is an ejected create-react-app that has been modified a fair bit.

@mattcosta7
Copy link
Contributor

Interesting! It looks like the code in undici is checking for the node version though, and avoiding it when the version is incorrect

Are your tests running in jsdom? Maybe that global isn't implemented in the version of jsdom you're using @robdodson - I think that could explain it not being available with the node version being correct?

@robdodson
Copy link
Author

It looks like we are using "jest-environment-jsdom": "^29.7.0" and we have testEnvironment: 'jest-environment-jsdom', set in our jest config.

jest-environment-jsdom v29.7.0 uses jsdom v20

jsdom v20.0.0 does have the performance object defined on window, but it does not implement markResourceTiming. Neither does the current version of jsdom.

I think markResourceTiming is a Node API, not a browser API, so it would make sense that jsdom would not implement it.

In jest.polyfills.js we set fetch to be undici's version but I'm a little confused as to where undici is looking when it tries to access performance.markResourceTiming — is it trying to access the performance object defined in jsdom or Node itself?

@robdodson
Copy link
Author

robdodson commented Nov 11, 2023

I've tried to reproduce the issue in this repo: https://github.com/robdodson/msw-test

I'm not seeing the same error, but I am seeing a different issue with undici trying to access a global that exists in Node but not JSDom:

console.warn
    [MSW] Warning: intercepted a request without a matching request handler:
    
      • GET /api/message
    
    If you still wish to intercept this unhandled request, please create a request handler for it.
    Read more: https://mswjs.io/docs/getting-started/mocks

      at Object.warn (node_modules/msw/lib/core/utils/internal/devUtils.js:31:11)
      at applyStrategy (node_modules/msw/lib/core/utils/request/onUnhandledRequest.js:170:36)
      at node_modules/msw/lib/core/utils/request/onUnhandledRequest.js:191:5
      at fulfilled (node_modules/msw/lib/core/utils/request/onUnhandledRequest.js:33:24)

 FAIL  src/App.test.tsx
  ✕ call fetch and render the message (1026 ms)

  ● call fetch and render the message

    ReferenceError: clearImmediate is not defined

      at node_modules/undici/lib/core/connect.js:180:5
      at Socket.<anonymous> (node_modules/undici/lib/core/connect.js:147:9)

I'm not entirely sure what's going on, but my rough guess is when we polyfill fetch with undici in jest.polyfills.js then undici starts trying to use the globals from JSDom. Neither performance.markResourceTiming nor clearImmediate exist in JSDom so undici hits runtime errors. Does that assumption sound correct?

@robdodson
Copy link
Author

robdodson commented Nov 11, 2023

Forgot to mention - in the repo, I specifically commented out my msw handlers to make the test fail. If you comment them back in, everything will work. So I'm guessing we're hitting a code path in undici when we 404 the request and it causes it to try to call clearImmediate

@mattcosta7
Copy link
Contributor

Forgot to mention - in the repo, I specifically commented out my msw handlers to make the test fail. If you comment them back in, everything will work. So I'm guessing we're hitting a code path in undici when we 404 the request and it causes it to try to call clearImmediate

interesting! definitely looks like that 404 / clearImmediate relates to node apis that jsdom doesn't implement.

The issue of "node" and "jsdom" implementations and how they kind of leak between eachother is...fun

@robdodson
Copy link
Author

Yeah, if the assumption is correct—that undici is trying to use missing globals in JSDom—then I'm wondering:

  • Should the migration guide docs on jest.polyfills.js be updated to include these two missing globals? I'm happy to submit a PR for that if folks agree that it's the right direction.
  • Should the migration guide recommend pinning a specific version of undici in package.json. Otherwise developers might need to keep adding polyfills if future versions of undici attempt to use other functions not defined in JSDom

@kettanaito what do you think?

@samanthaburboz
Copy link

@robdodson
Facing the same issue.

ReferenceError: clearImmediate is not defined

  at node_modules/undici/lib/core/connect.js:180:5
  at Socket.<anonymous> (node_modules/undici/lib/core/connect.js:147:9)

How do I add clearImmediate to the jest.polyfills.js?

@robdodson
Copy link
Author

@samanthaburboz Here's what my jest.polyfills.js looks like. I think performance and clearImmediate need to be imported and added to globalThis before undici is imported.

/**
 * @note The block below contains polyfills for Node.js globals
 * required for Jest to function when running JSDOM tests.
 * These HAVE to be require's and HAVE to be in this exact
 * order, since "undici" depends on the "TextEncoder" global API.
 *
 * Consider migrating to a more modern test runner if
 * you don't want to deal with this.
 */

const { performance } = require("node:perf_hooks");
const { TextDecoder, TextEncoder } = require("node:util");
const { clearImmediate } = require("node:timers");

Object.defineProperties(globalThis, {
  TextDecoder: { value: TextDecoder },
  TextEncoder: { value: TextEncoder },
  performance: { value: performance },
  clearImmediate: { value: clearImmediate },
});

const { Blob } = require("node:buffer");
const { fetch, Headers, FormData, Request, Response } = require("undici");

Object.defineProperties(globalThis, {
  fetch: { value: fetch, writable: true },
  Blob: { value: Blob },
  Headers: { value: Headers },
  FormData: { value: FormData },
  Request: { value: Request },
  Response: { value: Response },
});

@robdodson
Copy link
Author

robdodson commented Nov 20, 2023

I'm starting to think this might be a bigger issue and this polyfills file might make msw incompatible with Jest and libraries designed to work with Jest.

Here are some examples from our tests that I don't know how to work around:

  • React Testing Library user-event's clipboard API breaks because it attempts to use FileReader readAsText() which expects a Blob. Blob is defined in JSDom but we are replacing it with a version from Node that is not compatible.
TypeError: Failed to execute 'readAsText' on 'FileReader': parameter 1 is not of type 'Blob'.
Failed to execute 'append' on 'FormData': parameter 2 is not of type 'Blob'
  • jest.useFakeTimers({ legacyFakeTimers: true }) breaks because it attempts to use clearImmediate
TypeError: Cannot assign to read only property 'clearImmediate' of object '[object Window]'

If I comment out the polyfills then all of the above issues go away and the tests pass again.

edit:

Maybe a possible fix for the clearImmediate issue would be to make it writeable?

clearImmediate: { value: clearImmediate, writable: true },

But i'm not sure how to work around the other issues with FormData and Blob

@GitHelge
Copy link

GitHelge commented Nov 20, 2023

Thanks for the hint regarding adding the additional polyfills. However, the jest.useFakeTimers function breaks for me as well.

Running nodejs v18.16.0

    TypeError: Cannot assign to read only property 'performance' of object '[object Window]'

      79 |     });
      80 |     test('should do stuff', () => {
    > 81 |       jest.useFakeTimers({
         |            ^
      82 |         now: new Date('2023-10-31T00:00:00+01:00'),
      83 |         doNotFake: ['queueMicrotask'],
      84 |       });

Im currently thinking about migrating the project to vitest... I wonder what else breaks then ;-)

Edit:

configuring performance: { value: performance, writable: true }, as writable seems to help with jest.useFakeTimers

jest.useFakeTimers().setSystemTime(new Date('2023-01-01T00:00:00+01:00')); seems to work again... I have to little knowlegde about polyfills to say if this is a fix or a workaround. But all the tests are awefully slow. 27sec before, now it gets slower and slower 150s+. Maybe an unrelated problem.

@robdodson
Copy link
Author

If I make performance and clearImmediate writable, and remove Blob and FormData from the polyfills, then all of my tests pass again. Here's my current polyfills:

const { performance } = require('node:perf_hooks')
const { TextDecoder, TextEncoder } = require('node:util')
const { clearImmediate } = require('node:timers')

Object.defineProperties(globalThis, {
  TextDecoder: { value: TextDecoder },
  TextEncoder: { value: TextEncoder },
  performance: { value: performance, writable: true },
  clearImmediate: { value: clearImmediate, writable: true },
})

const { fetch, Headers, Request, Response } = require('undici')

Object.defineProperties(globalThis, {
  fetch: { value: fetch, writable: true },
  Headers: { value: Headers },
  Request: { value: Request },
  Response: { value: Response },
})

@mattcosta7 @kettanaito do either of you know why Blob and FormData were included in the original set of polyfills?

@mattcosta7
Copy link
Contributor

If I make performance and clearImmediate writable, and remove Blob and FormData from the polyfills, then all of my tests pass again. Here's my current polyfills:

const { performance } = require('node:perf_hooks')
const { TextDecoder, TextEncoder } = require('node:util')
const { clearImmediate } = require('node:timers')

Object.defineProperties(globalThis, {
  TextDecoder: { value: TextDecoder },
  TextEncoder: { value: TextEncoder },
  performance: { value: performance, writable: true },
  clearImmediate: { value: clearImmediate, writable: true },
})

const { fetch, Headers, Request, Response } = require('undici')

Object.defineProperties(globalThis, {
  fetch: { value: fetch, writable: true },
  Headers: { value: Headers },
  Request: { value: Request },
  Response: { value: Response },
})

@mattcosta7 @kettanaito do either of you know why Blob and FormData were included in the original set of polyfills?

I don't offhand know, my assumption would be consistency of the underlying stream behaviors?

@kettanaito
Copy link
Member

kettanaito commented Nov 21, 2023

@robdodson, yes, they were included in the original suggestion for jest.polyfills.js.

const { Blob } = require('node:buffer')
const { FormData } = require('undici')

We shouldn't remove them.

configuring performance: { value: performance, writable: true }, as writable seems to help with jest.useFakeTimers

Naturally, since writable: true allows re-defining that value, which is precisely what fake timers do.

React Testing Library user-event's clipboard API breaks because it attempts to use Blob readAsText which is defined in JSDom but not in Node's Blob

This isn't something you should care about. RTL runs exclusively in JSDOM. You say it yourself—that API is implemented in JSDOM. I fail to see the issue here.

I think performance and clearImmediate need to be imported and added to globalThis before undici is imported.

Thanks for noticing that order. It bit me quite a lot trying to figure out the right order of things for that polyfill file 👍

But all the tests are awefully slow. 27sec before, now it gets slower and slower 150s+

Very hard to reason about this. Using fake timers by design will affect test performance since you defer the test execution to the moment when you resolve the timers. Imo, not something MSW should be concerned about.

I'm starting to think this might be a bigger issue and this polyfills file might make msw incompatible with Jest and libraries designed to work with Jest.

A humble plug that you would spent 0 minutes on this if you used modern, supported testing framework, like Vitest. I'm okay with providing polyfills and suggestions on how to patch Jest to let you in on the modern JavaScript but this isn't something I'm planning on supporting at all.

@kettanaito
Copy link
Member

Also, a very important note: React Testing Library doesn't support Node.js v18. Inevitably, any attempts to run MSW (which requires Node.js v18+) with RTL will result in all sorts of odd behaviors.

With that in mind, here's my conclusion on this issue.

  1. It feels like a hack/workaround because it is. You are trying to polyfills missing Node.js APIs in the otherwise incompatible Node.js version for RTL/MSW. I'd rather you don't.
  2. Wait until RTL releases the next version that supports Node.js v18. It's the RTL's team responsibility to migrate to the LTS version of Node.js and make sure that all their packages are functional on that version.
  3. Once that happens, I believe no changes to the jest.polyfills.js will be needed.

@mattcosta7
Copy link
Contributor

React Testing Library user-event's clipboard API breaks because it attempts to use Blob readAsText which is defined in JSDom but not in Node's Blob

This isn't something you should care about. RTL runs exclusively in JSDOM. You say it yourself—that API is implemented in JSDOM. I fail to see the issue here.

I think the issues here are mostly around jsdom operation

Since jsdom intends to act as a browser and not as node - tests using jsdom call browser apis.

By polyfilling with node versions of these, were removing the ability to use browser like parts of them in jsdom - using node apis only.

Do/can we support browser based versions of these when running in jsdom? (I'm not actually sure offhand) but this might help avoid some issues generally.

Testing in jsdom is something I conceptually have trouble with because it's not but it's also trying very hard not to be - and we do want some node apis there but not all of them?

Tools designed to operate for jsdom like react testing library expect those dom like versions of these things (fetch, request, response,blob, form data) and maybe there's a better way to support them both without always relying on the node version?

@robdodson
Copy link
Author

By polyfilling with node versions of these, we're removing the ability to use browser like parts of them in jsdom - using node apis only.

Yes I think this is the core issue, not the lack of Node v18 support in RTL (that might be a separate issue, but I don't think it's the one I'm hitting in this ticket).

In my test I'm attempting to use the Clipboard API that RTL stubs out:

expect(await navigator.clipboard.readText()).toBe(expectedUrl.href)

Which throws this error from the convert() function in JSDom

TypeError: Failed to execute 'readAsText' on 'FileReader': parameter 1 is not of type 'Blob'.
  at Object.exports.convert (node_modules/jsdom/lib/jsdom/living/generated/Blob.js:23:9)
  at FileReader.readAsText (node_modules/jsdom/lib/jsdom/living/generated/FileReader.js:163:23)

Here is the convert() function in JSDom's Blob.js.

// node_modules/jsdom/lib/jsdom/living/generated/Blob.js

exports.is = value => {
  return utils.isObject(value) && utils.hasOwn(value, implSymbol) && value[implSymbol] instanceof Impl.implementation;
};
exports.isImpl = value => {
  return utils.isObject(value) && value instanceof Impl.implementation;
};
exports.convert = (globalObject, value, { context = "The provided value" } = {}) => {
  if (exports.is(value)) {  // <-- We are failing this check 🔴 
    return utils.implForWrapper(value);
  }
  throw new globalObject.TypeError(`${context} is not of type 'Blob'.`);
};

If I console.log the steps from is() then it shows that we are failing the second and third steps where it checks for the existence of a Symbol on the Blob

utils.isObject(value) // true
utils.hasOwn(value, implSymbol) // false
value[implSymbol] instanceof Impl.implementation // false

My guess is that when we replace JSDom's Blob with Node's Blob in jest.polyfills.js, it allows for the creation of Blobs that are missing a Symbol that JSDom normally attaches to them. When code tries to interact with the Blob in JSDom, it fails this identity check.

If I remove Blob from my jest.polyfills.js then the test passes. I suspect this happens because I'm not yet using any part of undici that depends on a Node-specific version of Blob.

Do/can we support browser based versions of these when running in jsdom? (I'm not actually sure offhand) but this might help avoid some issues generally.

I'm really curious to see how Vitest works around this? I'm going to try to recreate the issue with RTL in a Vitest app to see if everything "just works".

@github-actions github-actions bot locked and limited conversation to collaborators Nov 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node
Projects
None yet
Development

No branches or pull requests

5 participants