Skip to content

Migrate to esbuild - #5589

Merged
mstange merged 1 commit into
firefox-devtools:mainfrom
mstange:esbuild
Feb 14, 2026
Merged

Migrate to esbuild#5589
mstange merged 1 commit into
firefox-devtools:mainfrom
mstange:esbuild

Conversation

@mstange

@mstangemstange commented Sep 3, 2025

Copy link
Copy Markdown
Contributor

@codecov

codecovBot commented Jan 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.56%. Comparing base (ca86724) to head (c0b82be).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #5589 +/- ##
==========================================
+ Coverage 85.55% 85.56% +0.01% 
==========================================
Files 320 319 -1 Lines 31415 31411 -4 Branches 8662 8670 +8 ==========================================
Hits 26876 26876 + Misses 4108 4104 -4 
Partials 431 431 

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

@mstange
mstangeforce-pushed the esbuild branch 4 times, most recently from 4ffc248 to 03f4d99CompareJanuary 20, 2026 16:41
@mstange
mstangeforce-pushed the esbuild branch 6 times, most recently from 9e3eff2 to 31d83b2CompareFebruary 4, 2026 21:56
@mstange
mstange marked this pull request as ready for review February 4, 2026 22:01
@mstange
mstange requested a review from canova as a code ownerFebruary 4, 2026 22:01
@mstangemstange changed the title WIP: Migrate to esbuildMigrate to esbuildFeb 4, 2026
@mstange
mstange marked this pull request as draft February 5, 2026 14:51
@mstange
mstange removed the request for review from canovaFebruary 5, 2026 14:51
@mstange
mstangeforce-pushed the esbuild branch 3 times, most recently from a9cf93b to bd3f05aCompareFebruary 5, 2026 15:54
@mstange
mstangeforce-pushed the esbuild branch 3 times, most recently from f7b5e5f to 3cee9ecCompareFebruary 5, 2026 17:04
@mstange
mstange marked this pull request as ready for review February 5, 2026 17:04
@mstange
mstange requested a review from canovaFebruary 6, 2026 18:58
Comment threadscripts/lib/build-utils.mjs Outdated
}
}

export function saveMetafile(buildResult, outputPath = 'dist/metafile.json') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is metafile exactly? I see that when we build, it puts the meta file to the metafile.json to the dist directory. Should we remove it at the end? Will it be exposed to web if it gets uploaded?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good question. It might make sense to have nested directories under dist/ for the various things.

E.g.:

  • dist/browser-build-prod/...
  • dist/meta/browser-build-prod.json
  • dist/browser-build-dev/...
  • dist/node/symbolicator-cli.js

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually let's just keep the directory structure as-is.

You're right, the metafile was making it into the production output, which definitely was not what I intended. Let's stop emitting it by default; we don't need it, it's just there in case you want to do some bundle size profiling. And when we do emit it, we can put it outside of dist, for example into a new build-meta directory.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, I would prefer to keep the dist directory as is too. Sounds good to move this file out of dist.

Comment on lines +111 to +120
export const photonConfig = {
...baseConfig,
format: 'esm',
platform: 'browser',
target: ['es2022'],
sourcemap: true,
publicPath: '/photon/',
entryPoints: ['res/photon/index.js'],
outdir: 'dist/photon',
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently yarn start-photon shows me 404 when I visit http://localhost:4243/photon/ . I think we don't copy the photon/index.html file. Could you fix that?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, will do!

@canovacanova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

I did some comparisons in the before and after of the dist/ directory after yarn build-prod and I can already see some nice improvements:

The total size of directory went from ~23M to ~22M.
Main bundle size went from ~1.6M and ~1.1M
I see total JS bundle size went from ~2.3M ~1.4M.

I think the main difference of the JS size difference is that webpack bundles the CSS with JS, and now CSS is moved to its own CSS file. I think it's a nice improvement.

Apart from that, I see that we stopped auto prefixing (and possibly minifying) the CSS though. See my comment below in postcss.config.js. What do you think?


// Plugin to mark chrome:// URLs as external in CSS
//
// This is a workaround for https://bugzilla.mozilla.org/show_bug.cgi?id=2014811

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah looks like it's resolved but not published yet. It would be nice to file an issue to clean this up later.

Comment threadscripts/lib/esbuild-plugins.mjs Outdated

for (const modulePath of Object.keys(inputs)) {
if (!modulePath.includes('node_modules')) {
findCycles(modulePath, []);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be like this?:

Suggested change
findCycles(modulePath,[]);
findCycles([],modulePath);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops! Indeed!

Comment threadpostcss.config.js
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
module.exports = {
plugins: [require('cssnano'), require('autoprefixer')],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I don't see postcss anymore in the codebase, but we still have it in the package.json. Did we stop transforming the CSS now?

Edit: Checking the css, I think we stopped prefixing the css properties now with autoprefixer for webkit etc. For example, I don't see -webkit- equivalent of user-select: none;. I think this might create a real compat issue in Safari. What do you think? I see that there is esbuild-plugin-postcss if we want to have it still.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh indeed, without the webkit prefixed version, I can see text selection happening e.g. in the "tabs" when I right-click them. I'll add the postcss plugin.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than using the postcss plugin, I chose https://www.npmjs.com/package/browserslist-to-esbuild and replaced target: 'es2022' with target: browserslistToEsbuild(); specifying browser versions rather than ECMAScript versions will cause esbuild to do the prefixing for us, see https://esbuild.github.io/content-types/#css .

@canovacanova left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me overall, thanks again for the work! I'll let you handle the landing after you look into the postcss stuff.

@mstange
mstange enabled auto-merge February 14, 2026 17:16
@mstange
mstange merged commit 7304276 into firefox-devtools:mainFeb 14, 2026
19 checks passed
@canovacanova mentioned this pull request Feb 24, 2026
canova added a commit that referenced this pull request Feb 24, 2026
Changes:
[Markus Stange] Move transparent fill check to a less expensive place.
(#5826)
[fatadel] refine docs for ctrl key (#5831)
[Nazım Can Altınova] Update the team members inside CONTRIBUTING.md
(#5829)
[fatadel] Provide correct instructions on the home page for Firefox for
Android (Fenix) (#5816)
[Markus Stange] Make markers darker (#5839)
[Markus Stange] Directly use photon-colors (#5821)
[Markus Stange] Migrate to esbuild (#5589)
[Markus Stange] Use data URLs for SVG files. (#5845)
[Markus Stange] Tweak dark mode colors some more. (#5846)
[Nazım Can Altınova] Make the source table non-optional in the gecko
profile format (#5842)
[fatadel] fix foreground color of the button on error page (#5849)
[Nazım Can Altınova] 🔃 Sync: l10n -> main (February 24, 2026) (#5855)
And special thanks to our localizers:
de: Michael Köhler
el: Jim Spentzos
en-GB: Ian Neal
es-CL: ravmn
fr: Théo Chevalier
fy-NL: Fjoerfoks
ia: Melo46
it: Francesco Lodolo [:flod]
nl: Mark Heijl
ru: Valery Ledovskoy
sv-SE: Andreas Pettersson
zh-TW: Pin-guang Chen
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mstange@canova