Update fetch-mock-jest to @fetch-mock/jest by julienw · Pull Request #5488 · firefox-devtools/profiler

julienw · 2025-06-11T12:59:22Z

This PR updates the obsolete fetch-mock-jest to its new version @fetch-mock/jest.
Unfortunately this comes with lots of syntax changes. In particular, previously fetch was both the mock and the configuration object, but now they're 2 different objects.

There's a useful migration page: https://www.wheresrhys.co.uk/fetch-mock/docs/Usage/upgrade-guide

The fact this patch also removes node-fetch brought other problems, I believe due to the interaction with jsdom or jest's vm mechanism, with different cross-realm constructors, so I had to minimally change some app code as well.

Also due to jsdom's not implementing fetch and other fetch-related objects that are now available in nodejs by default (even if not exactly the same), I had to supply a custom environment.

I tried to split the PR by logical commits.

Tell me what you think!

julienw · 2025-06-11T13:00:53Z

src/test/setup.js

+fetchMock.mockGlobal();
+global.fetchMock = fetchMock;


I was afraid that this could be an issue when running tests in parallel, but I couldn't make it fail. I think that nowadays jest (or is it the jsdom environment, or just new versions of jsdom) isolates the globals better than it used to.

codecov · 2025-06-11T13:08:06Z

Codecov Report

Attention: Patch coverage is 38.09524% with 13 lines in your changes missing coverage. Please review.

Project coverage is 86.11%. Comparing base (452ec65) to head (7aa6c5f).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/test/custom-environment.js	0.00%	10 Missing and 2 partials ⚠️
src/profile-logic/mozilla-symbolication-api.js	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5488      +/-   ##
==========================================
- Coverage   86.15%   86.11%   -0.04%     
==========================================
  Files         307      308       +1     
  Lines       29637    29652      +15     
  Branches     7999     8001       +2     
==========================================
+ Hits        25533    25536       +3     
- Misses       3516     3526      +10     
- Partials      588      590       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

julienw · 2025-06-11T13:49:23Z

src/test/custom-environment.js

+    // in jest apparently.
+    // Still let's double check that they're from the global scope as expected, so
+    // that this can be removed once it's implemented.
+    if ('TextDecoder' in this.global) {


I'm not fully sure this condition works, TBH... I don't know if this.global contains all the globals, or contains only the new things.

In perfcompare I'm using a newer jsdom that has Request, the followng check actually worked, so it looks like this works after all.

I see jest v30 was just released 2 days ago, this file will probably need to be adjusted with that update.

In perfcompare I'm using a newer jsdom that has Request, the followng check actually worked, so it looks like this works after all.

Nice, thanks for checking!

I see jest v30 was just released 2 days ago, this file will probably need to be adjusted with that update.

Ah, do we have TextDecoder in v30?

TextDecoder I don't know, but Request, Response, fetch are there (but not ReadableStream surprisingly!)

julienw · 2025-06-12T14:21:42Z

src/test/custom-environment.js

+import { TestEnvironment } from 'jest-environment-jsdom';
+import { TextDecoder, TextEncoder } from 'util';
+
+export default class CustomTestEnvironment extends TestEnvironment {


This comes from https://www.wheresrhys.co.uk/fetch-mock/docs/wrappers/jest#jsdom-campatibility, adapted for our use case.

canova

Looks good to me. But wow, that's surprisingly a lot of changes. I always get annoyed when a utility/testing package has this big of a breaking change (insert "old man yells at cloud" meme). To be fair, technically it's a different package, but still... At least I'm happy that I didn't have to deal with this, so thank you! 😄

I have some questions but they are mostly so I understand better, not blocking.

canova · 2025-06-13T16:39:17Z

package.json

    ],
    "transformIgnorePatterns": [
-      "/node_modules/(?!(query-string|decode-uri-component|split-on-first|filter-obj)/)"
+      "/node_modules/(?!(query-string|decode-uri-component|split-on-first|filter-obj|@fetch-mock/jest|fetch-mock)/)"


Hm I can't seem to remember this. Why do we need this one?

Because our current configuration doesn't deal with esm import and export when running in jest (I think it should be possible to make it work easily nowadays...).

This configuration property configures jest to not run babel on some files, by default this is all of node_modules. We override this line to run babel on some npm packages that are using export/import.

canova · 2025-06-13T16:49:09Z

src/test/custom-environment.js

+    // in jest apparently.
+    // Still let's double check that they're from the global scope as expected, so
+    // that this can be removed once it's implemented.
+    if ('TextDecoder' in this.global) {


In perfcompare I'm using a newer jsdom that has Request, the followng check actually worked, so it looks like this works after all.

Nice, thanks for checking!

I see jest v30 was just released 2 days ago, this file will probably need to be adjusted with that update.

Ah, do we have TextDecoder in v30?

src/actions/receive-profile.js

canova · 2025-06-13T16:58:50Z

src/profile-logic/mozilla-symbolication-api.js

+  const isObject = (subject) =>
+    Object.prototype.toString.call(subject) === '[object Object]';


Why did we need a function like this?

I think the problem is that the object we get inherits from the Object object in another js environment.
Also the previous code didn't work for an object without a prototype (such as when created with Object.create(null)).

I'll add a comment :)

I kind of wonder if this might be a bug in jest though.

I've seen this in jest's node environment => https://github.com/jestjs/jest/blob/42c8e7de64d1a3f8ee9b7f3b309332ac8f88e8b1/packages/jest-environment-node/src/index.ts#L157-L161
but it's not in the jsdom environment.
This might be a red herring though.

…ross-realm prototypes

Changes: [Nazım Can Altınova] Fix issues related to the track borders (#5484) [Nazım Can Altınova] Remove the active tab and origins views (#5483) [Julien Wajsberg] Automatically request reviews for dependency updates (#5490) [Julien Wajsberg] Update fetch-mock-jest to @fetch-mock/jest (#5488) [Steve Fink] Document marker filter syntax (#5493) [Nazım Can Altınova] Update all of jest 29.7.0 → 30.0.0 (major) (#5495) [Nazım Can Altınova] Order global tracks by activity and select the most active non-parent process by default (#5491) [Paul Adenot] Allow searching for HTTP response status in marker views (#5504) [Nazım Can Altınova] Expose a `totalMarkerDuration` function in console (#5507) [Nazım Can Altınova] 🔃 Sync: l10n -> main (July 11, 2025) (#5510) And thanks to our localizers: el: Jim Spentzos kab: ZiriSut tr: Grk tr: Selim Şumlu uk: Artem Polivanchuk

julienw commented Jun 11, 2025

View reviewed changes

julienw requested a review from mstange June 11, 2025 13:01

julienw commented Jun 11, 2025

View reviewed changes

julienw requested a review from canova June 12, 2025 12:13

julienw commented Jun 12, 2025

View reviewed changes

canova approved these changes Jun 13, 2025

View reviewed changes

julienw added 5 commits June 16, 2025 10:56

Update fetch-mock-jest to @fetch-mock/jest and remove node-fetch

f044dd8

Configure the new fetchMock

fa3a2be

Update the tests to follow the new syntax

4ec11d9

Additional changes due to removing node-fetch, bringing issues with c…

d4d8ee6

…ross-realm prototypes

Add some more comments

7aa6c5f

julienw force-pushed the update-fetch-mock branch from 3f5f7e5 to 7aa6c5f Compare June 16, 2025 09:17

julienw merged commit 530446e into firefox-devtools:main Jun 16, 2025
13 of 15 checks passed

canova mentioned this pull request Jul 11, 2025

Deploy July 11, 2025 #5511

Merged

		const isObject = (subject) =>
		Object.prototype.toString.call(subject) === '[object Object]';

		fetchMock.mockGlobal();
		global.fetchMock = fetchMock;

Conversation

julienw commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

canova left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

julienw commented Jun 11, 2025 •

edited

Loading

codecov bot commented Jun 11, 2025 •

edited

Loading