Convert the entire codebase to TypeScript#5549

mstange · 2025-08-08T03:54:41Z

I squashed and re-split the changes from #5531.

yarn test-all passes after every commit.

702 files changed, 13512 insertions(+), 64132 deletions(-)

codecov · 2025-08-08T04:01:32Z

Codecov Report

❌ Patch coverage is 87.79586% with 330 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.84%. Comparing base (333baf8) to head (16260f2).

Files with missing lines	Patch %	Lines
src/actions/receive-profile.ts	87.14%	68 Missing ⚠️
src/actions/profile-view.ts	91.93%	50 Missing and 2 partials ⚠️
src/app-logic/url-handling.ts	90.25%	48 Missing and 1 partial ⚠️
src/components/app/CodeErrorOverlay.tsx	0.00%	32 Missing ⚠️
src/actions/app.ts	75.23%	26 Missing ⚠️
src/app-logic/browser-connection.ts	73.84%	17 Missing ⚠️
src/components/app/BottomBox.tsx	70.90%	13 Missing and 3 partials ⚠️
src/components/app/AppViewRouter.tsx	79.59%	10 Missing ⚠️
src/actions/code.ts	50.00%	8 Missing ⚠️
src/app-logic/web-channel.ts	89.18%	8 Missing ⚠️
... and 12 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5549      +/-   ##
==========================================
- Coverage   86.13%   85.84%   -0.30%     
==========================================
  Files         309      309              
  Lines       29883    30319     +436     
  Branches     8038     8345     +307     
==========================================
+ Hits        25740    26026     +286     
- Misses       3552     3872     +320     
+ Partials      591      421     -170

☔ 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.

canova · 2025-08-08T15:38:50Z

package.json

-    "flow-stop": "flow stop",
-    "flow-coverage": "npx flow-coverage-report -i 'src/**/*.js' -t html -t text",
-    "flow-generate-libdefs": "npx flow-typed install --libdefDir src/types/libdef",
+    "typecheck": "tsc --noEmit",


nit: can we maybe name this ts? yarn ts is shorter and easier to type for me. And we already have the same command in profiler-server.

Absolutely!

I've added a commit to do the rename.

canova

Oh wow, this PR is huge, amazing work! I couldn't finish the review yet, I'm at ecff67d (Convert window-console.js and index.js.) at the moment. At least all the shipping code is done! Tomorrow I will continue with the rest. I'm submitting the comments that I wrote so far, so you can look at my them sooner than later. I expect to finish the review tomorrow.

I see a few of places where I see flow being mentioned. But I think they can be removed as a follow-ups.

For example:

We still have utils/flow.ts. It's funny that it's flow.ts now, maybe we can change it to utils/typescript.ts or utils/type.ts
I see some flow mentions inside src/reducers/README.md
Some more flow mentions in the comments.

But they all could be done as follow-ups.

Some files for some reason don't appear they are git mved even though they are in the same commit, but that's fine I think.

Also as a follow-up, it might be good to add the commit hashes of these inside .git-blame-ignore-revs so they would be ignored from the blame? (don't know if adding the merge commit hash is enough or we need to add every one of them individually)

Ah lastly, can you run npx yarn-deduplicate to deduplicate some of the packages that we added?

package.json

canova · 2025-08-12T18:48:04Z

package.json

-    "flow-coverage": "npx flow-coverage-report -i 'src/**/*.js' -t html -t text",
-    "flow-generate-libdefs": "npx flow-typed install --libdefDir src/types/libdef",
-    "protoc": "npx -p protobufjs-cli pbjs -t static-module -w commonjs -o ./src/profile-logic/import/proto/simpleperf_report.js ./src/profile-logic/import/proto/simpleperf_report.proto",
+    "ts": "tsc --noEmit",


We already have noEmit: true inside tsconfig.json, we can remove this one.

canova · 2025-08-12T18:51:52Z

tsconfig.json

+
+    // Exceptions to strictness, consider enabling in the future
+    "useUnknownInCatchVariables": false, // Sounds fine, enforces `} catch (e) { if (e instanceof Error) {` etc
+    "alwaysStrict": false, // Affects runtime behavior, I think - need to check what we had with Flow


Doc says: Ensures that your files are parsed in the ECMAScript strict mode, and emit “use strict” for each source file.

Since we have "noEmit": true, we are not actually using tsc itself for transpiling the source into JS, and rely on babel. So I think this option shouldn't have any effect.

I actually removed this line and compiled the source code again and main js bundle file has the same hash.

Ah of course, makes sense. I'll remove it.

canova · 2025-08-12T18:55:25Z

tsconfig.json

+    // Exceptions to strictness, consider enabling in the future
+    "useUnknownInCatchVariables": false, // Sounds fine, enforces `} catch (e) { if (e instanceof Error) {` etc
+    "alwaysStrict": false, // Affects runtime behavior, I think - need to check what we had with Flow
+    "exactOptionalPropertyTypes": false, // Benefit unclear


Hm, I'm not so sure about this, removing it doesn't throw any error, but also don't know if it makes much difference.

Oh, it seems to default to false even in "strict": true mode, so removing this line doesn't enable the flag.

canova · 2025-08-12T18:59:27Z

tsconfig.json

+
+    // Module Resolution
+    "resolveJsonModule": true,
+    "baseUrl": ".",


Do we need this?

seems not, I'll remove it.

canova · 2025-08-12T19:37:37Z

src/utils/bisect.ts

-  low?: number,
-  high?: number
+  low: number = 0,
+  high: number = array.length


Whattt, I don't know why I didn't know this, but I've never tried referencing another parameter inside the following parameters in JS before (like = array.length). I just assumed it wouldn't work. I guess? TIL!

Haha yes, this is one of the few times where this is quite handy!

canova · 2025-08-12T20:01:50Z

src/selectors/app.tsx

+  getApp(state).trackThreadHeights as unknown as {
+    [key: ThreadsKey]: CssPixels;
+  };


nit: Removing the casting still passed the ts check for me:

export const getTrackThreadHeights: Selector<{ [key: ThreadsKey]: CssPixels; }> = (state) => getApp(state).trackThreadHeights;

oh thanks for catching that

canova · 2025-08-12T20:03:34Z

src/actions/app.ts

      });
      dispatch({
-        type: 'CHANGE_SELECTED_TAB',
+        type: 'CHANGE_SELECTED_TAB' as const,


oh curious, why do we need to pass them as const?

hmm, it seems I don't need to! Maybe I converted this file before I had the Action type properly set up.

I've removed the as consts.

canova · 2025-08-12T20:14:08Z

src/components/shared/SampleTooltipContents.tsx

-|};
-
-type Props = {|
-  ...RestProps,


Is it not possible to use ...RestProps, in ts? Hm, I see & SizeProps in some other places.

The trouble here was the sampleIndex prop, which is | null for Props but required in RestProps.
If I do type Props = { <nonrestprops> } & RestProps thensampleIndex also becomes required in Props.

canova · 2025-08-12T20:19:06Z

src/components/tooltip/TrackPower.tsx

            'TrackPower--tooltip-power-watt',
-            'TrackPower--tooltip-power-milliwatt'
+            'TrackPower--tooltip-power-milliwatt',
+            'TrackPower--tooltip-power-microwatt'


Ah, I guess we didn't have this TrackPower--tooltip-power-microwatt l10n id before. It would be good file a bug for that.

oops, well, we still don't have it. Filed #5554 and removed the change. I changed the type of the last argument to _formatPower to be optional.

canova

I'm done going through the commits! It was easier to look at the second half.

Wow, thanks a lot for the migration! Looks great. Let's land this and we can follow-up with the improvements.

FWIW, I was using this branch locally to test stuff too and I can really feel the difference between flow and this! It was taking a long time to get LSP errors on my editor with flow, now I'm getting near instant feedback on errors! The experience difference is night and day. And after this, I can finally remove every config/plugin related to Flow from my editor.

canova · 2025-08-13T15:42:51Z

src/test/fixtures/mocks/window-navigation.ts

 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
-
-// @flow
+// @ts-nocheck Complex window/history mock with extensive DOM API manipulation that requires intricate typing


Not really worth a look now, but it could be interesting to see if we can do it somehow (not for this PR, some other time)

canova · 2025-08-13T15:43:44Z

src/test/fixtures/utils.ts

 export function fireFullKeyPress(
  element: HTMLElement,
-  options: { key: string, ... }
+  options: { key: string }


Heh non exact typing comes in handy here.

canova · 2025-08-13T15:51:02Z

src/test/types/react-redux.tsx

+/**
+ * These type tests create various values that should all type check correctly to show
+ * that the react-redux system is working correctly.
+ *
+ * We are catching the following mistakes at the type level:
+ *
+ *                            explicitConnect           plain connect
+ * mapStateToProps:
+ * - missing prop             yes                       yes
+ * - prop with wrong type     yes                       yes
+ * - extra prop               no (was yes with Flow)    no
+ *
+ * mapDispatchToProps:
+ * - missing prop             yes                       yes
+ * - action wrong type        yes                       yes
+ * - thunk action wrong type  yes                       yes
+ * - extra prop               no (was yes with Flow)    no
+ *
+ * OwnProps:
+ * - missing prop             yes                       yes
+ * - prop with wrong type     yes                       yes
+ * - extra prop               yes                       yes
+ *
+ * In summary, we lost the ability to detect unused state props and unused
+ * dispatch props when we migrated to TypeScript. On the other hand,
+ * explicitConnect now catches the same mistakes as plain connect, so we
+ * can migrate to plain connect at any time without losing type coverage.
+ */


Thanks for the comment! I guess that was the reason you mentioned #3063?

canova · 2025-08-13T15:55:21Z

src/test/unit/path.test.ts

    expect(Array.from(set)).toEqual(sampleValues);
-    expect(Array.from(set.values())).toEqual(sampleValues);
+    // @ts-expect-error The TypeScript definition of Array.from only accepts Iterable, not Iterator - not sure what's correct here
+    expect(Array.from<CallNodePath>(set.values())).toEqual(sampleValues);


Huh, that's weird, but I have no idea either.

canova · 2025-08-13T15:58:11Z

src/test/components/AppLocalizationProvider.test.tsx

        const matchUrlResult = fetchUrlRe.exec(url);
        if (matchUrlResult) {
-          // $FlowExpectError Our Flow doesn't know about named groups.


canova · 2025-08-13T16:05:48Z

src/test/url-handling.test.ts

Heh, I agree that this doesn't look like a correct location for this test (not for this PR again ofc).

I had to add two "resolutions" to the package.json: - "@types/react-splitter-layout/@types/react": "^18.3.1" so that we don't end up with two different typings for React. - "@types/trusted-types": "^2.0.7" because workbox depends on "^2.0.2" which has conflicts with the "official" TypeScript DOM types, and this conflict was fixed in v2.0.3. v2.0.7 is just the most recent version, so I picked that.

…nd src/utils.

This test looks like it wanted to be in some directory but couldn't find the right door. It seems a bit lost.

…onfig.json.

…ing babel to emit the JS.

mstange · 2025-08-14T18:56:25Z

Thanks for the thorough review of this huge PR!

I see a few of places where I see flow being mentioned. But I think they can be removed as a follow-ups.

I ended up fixing them all.

For example:

* We still have `utils/flow.ts`. It's funny that it's flow.ts now, maybe we can change it to utils/typescript.ts or utils/type.ts

Renamed to utils/types.ts. I don't like the file very much - I think files under utils should be the "leaves" of the module dependency tree, and not depend on type definitions for our actions etc. I think functions like toValidTabSlug should be close to where we define the TabSlug type, etc.

* I see some flow mentions inside src/reducers/README.md

Fixed, and I went through the other .md files as well.

* Some more flow mentions in the comments.

Oh actually this one I haven't fixed.

Some files for some reason don't appear they are git mved even though they are in the same commit, but that's fine I think.

Yeah I'm not sure why. Maybe because those didn't hit the percentage threshold of preserved lines.

Also as a follow-up, it might be good to add the commit hashes of these inside .git-blame-ignore-revs so they would be ignored from the blame? (don't know if adding the merge commit hash is enough or we need to add every one of them individually)

Ah yes, good idea.

Ah lastly, can you run npx yarn-deduplicate to deduplicate some of the packages that we added?

Done.

FWIW, I was using this branch locally to test stuff too and I can really feel the difference between flow and this! It was taking a long time to get LSP errors on my editor with flow, now I'm getting near instant feedback on errors! The experience difference is night and day. And after this, I can finally remove every config/plugin related to Flow from my editor.

🎉

Changes: [Nazım Can Altınova] Display the marker description at the top inside the marker tooltips (#5534) [Florian Quèze] Change the 'JavaScript' radio button label to 'Script' (#5530) [Markus Stange] Implement profile logic and some selectors for the function list (#5525) [Markus Stange] Some small type fixes (#5538) [Markus Stange] Simplify return type of the callback we pass to setState. (#5540) [Markus Stange] Pass the correct value to the reducer's action argument (#5543) [Markus Stange] Change withSize to accept PropsWithoutSize as its type parameter (#5541) [Nazım Can Altınova] Make sure that the test-debug command runs the tests properly (#5545) [Markus Stange] Improve type coverage involving network phases (#5539) [Markus Stange] Change implementation of withChartViewport (#5542) [Florian Quèze] A new permalink should be generated and shown after using the re-upload feature. (#5547) [Florian Quèze] Show the vertical ruler in the timeline when hovering the network chart (#5548) [Markus Stange] Convert the entire codebase to TypeScript (#5549) [Nazım Can Altınova] Update the yarn.lock file after recent changes (#5557) [Markus Stange] Add proper TypeScript coverage for window-navigation.ts (#5559) [Markus Stange] Remove leftover $FlowExpectError comments (#5560) [Markus Stange] Fix Iterator / Iterable confusion (#5561) [Nazım Can Altınova] Remove the unneeded test-all:ci script (#5566) [Nazım Can Altınova] Fix a type case inconsistency (#5569) [Florian Quèze] Make 'yarn lint --fix' work as an alias to 'yarn lint-fix'. (#5563) [Ryan Hunt] Don't stringify JSON again in fetchUrlResponse (#5570) [Nazım Can Altınova] Upgrade ESLint to version 9 (#5567) [Markus Stange] Simplify Worker setup, and support .json.gz inputs in symbolicator-cli (#5556) [Nazım Can Altınova] Add TypeScript coverage to the intersection observer mock (#5574) [Markus Stange] Set the preview selection to null when there is no selection (#5568) [Markus Stange] Add tests for query-api.ts (#5571) [Markus Stange] Enable noUnusedParameters and @typescript-eslint/no-unused-vars and clean up a few more things (#5576) [Ryan Hunt] Embed iongraph-web and use for iongraph.json source files (#5577) [Markus Stange] Remove recursion in filterThreadToSearchString (#5572) And thanks to our localizers: be: Mikalai Udodau de: Michael Köhler el: Jim Spentzos en-CA: chutten en-GB: Paul es-CL: ravmn fr: Théo Chevalier fur: Fabio Tomat fy-NL: Fjoerfoks ia: Melo46 it: Francesco Lodolo [:flod] nl: Mark Heijl pt-BR: Marcelo Ghelman ru: Valery Ledovskoy sv-SE: Luna Jernberg tr: Rua tr: Selim Şumlu uk: Іhor Hordiichuk zh-CN: Olvcpr423 zh-TW: Pin-guang Chen

Example with Focus function transform: [Production](https://share.firefox.dev/4mW78M3) | [Deploy preview](https://deploy-preview-5613--perf-html.netlify.app/public/ggqb1y28h0nmwyxaqjdhvrnmfpn58hyeg25cza8/calltree/?globalTrackOrder=q0phkm7in1c9f5g862boela34dj&hiddenGlobalTracks=1wo&hiddenLocalTracksByPid=2465-1w4~92294-0~36702-0~2523-0~36259-0w2~71663-01~71664-0&search=oae&thread=x6&transforms=ff-280&v=11) Example with JS-only view: [Production](https://share.firefox.dev/4poRI4M) | [Deploy preview](https://deploy-preview-5613--perf-html.netlify.app/public/rpnres9s7rdrsfb240wtskvrq5sv7xzn3f4m2p0/calltree/?globalTrackOrder=30w2&hiddenGlobalTracks=01&hiddenLocalTracksByPid=2588-1w3~2591-01&implementation=js&range=9133m2135~9517m70&search=aoeu&tabID=25&thread=f&v=11) Fixes #5612. The more stringent array bounds checks from #5599 revealed a bug that I introduced in #5549: Marker stacks were not being substituted if the `convertStack` lambda returned null. This PR fixes the type and restores the working code from before the TypeScript migration.

mstange requested a review from canova as a code owner August 8, 2025 03:54

mstange force-pushed the typescript-migration branch 8 times, most recently from 60d093b to 454f419 Compare August 8, 2025 04:36

canova reviewed Aug 8, 2025

View reviewed changes

mstange force-pushed the typescript-migration branch 3 times, most recently from 9b9eeee to 16260f2 Compare August 11, 2025 21:39

canova mentioned this pull request Aug 12, 2025

Display traced values in Stack Chart view #5363

Merged

canova reviewed Aug 12, 2025

View reviewed changes

canova approved these changes Aug 13, 2025

View reviewed changes

mstange added 13 commits August 13, 2025 21:30

Update eslintrc

971a6a3

Remove .js extension from imports.

9a09f31

Add TypeScript types for various dependencies.

c323b62

Add handwritten types for dependencies which don't have types in @types.

6d93997

Convert some files under src/types/.

806318d

Convert some files under src/utils.

8ccd2d2

Convert more files under src/types.

2c89925

Convert more files under src/utils.

ce97bc1

Convert src/types/mocks/ftl.ts.

2b88259

Convert src/app-logic, src/profile-logic, and the rest of src/types a…

ed02f3f

…nd src/utils.

Convert src/reducers.

066d548

Convert src/selectors.

9b82bcd

mstange added 12 commits August 14, 2025 14:31

Convert src/test/fixtures.

5ef2034

Setup for TypeScript tests.

325c2f2

Convert test/types.

19a9bbd

Convert test/unit.

2a13ee1

Convert test/store.

42e4810

Convert test/components.

98c6157

Convert test/integration.

caa1bc6

Convert test/url-handling.test.js.

43ddbd5

This test looks like it wanted to be in some directory but couldn't find the right door. It seems a bit lost.

Convert __mocks__.

227668a

Remove src/types/libdef.

d847d69

Remove various flow remnants.

504f153

Remove @flow and @noflow annotations.

fcf4ee7

mstange force-pushed the typescript-migration branch from 16260f2 to 8ba9ac4 Compare August 14, 2025 18:42

mstange added 8 commits August 14, 2025 14:48

Update readme files.

6dc238e

Rename yarn typecheck to yarn ts.

26ab457

Remove unnecessary --noEmit - we already have noEmit: true inside tsc…

25ec877

…onfig.json.

Remove "alwaysStrict": false; it makes no difference because we're us…

9914eb5

…ing babel to emit the JS.

Remove unnecessary "baseUrl" prop from tsconfig.

d9ec496

Move global.d.ts into src/types/globals.

37daba7

Run npx yarn-deduplicate.

8840445

Rename src/utils/flow.ts to types.ts.

fe67997

mstange force-pushed the typescript-migration branch from 8ba9ac4 to fe67997 Compare August 14, 2025 18:48

mstange merged commit 44b7b85 into firefox-devtools:main Aug 14, 2025
13 checks passed

mstange mentioned this pull request Aug 14, 2025

Prototype TypeScript migration with Claude #5531

Closed

canova mentioned this pull request Sep 2, 2025

Deploy September 2, 2025 #5580

Merged

mstange mentioned this pull request Sep 11, 2025

Fix null marker stack substitution #5613

Merged

fatadel mentioned this pull request Jan 30, 2026

add TrackPower--tooltip-power-microwatt #5799

Merged

Conversation

mstange commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Aug 8, 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!

canova left a comment

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!

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!

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!

mstange Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

mstange commented Aug 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

mstange commented Aug 8, 2025 •

edited

Loading

codecov bot commented Aug 8, 2025 •

edited

Loading

mstange Aug 14, 2025 •

edited

Loading