Skip to content

Conversation

@mstange
Copy link
Contributor

@mstange mstange commented Nov 16, 2025

Production | Deploy preview

This replaces the webpack plugin with an invocation of the workbox CLI binary.

@codecov
Copy link

codecov bot commented Nov 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.62%. Comparing base (abd8ccf) to head (0f30a98).
⚠️ Report is 48 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5681   +/-   ##
=======================================
  Coverage   85.62%   85.62%           
=======================================
  Files         312      312           
  Lines       30875    30879    +4     
  Branches     8507     8501    -6     
=======================================
+ Hits        26436    26440    +4     
  Misses       4009     4009           
  Partials      430      430           

☔ 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 mstange marked this pull request as ready for review November 16, 2025 03:38
@mstange mstange requested a review from canova as a code owner November 16, 2025 03:38
Copy link
Member

@canova canova left a comment

Choose a reason for hiding this comment

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

Hmm, can you tell me more about the reason why this was needed? I think code-wise it looks good to me, but I want to understand the motivation behind it, because I feel like workbox-webpack-plugin is a more convenient way to use workbox as long as there is no strong need for it (also npm download numbers show that the webpack plugin is way more popular than the cli). One thing I suspect is the pq cli tool?

Comment on lines +16 to +17
"build-prod:quiet": "yarn build:clean && yarn build-photon && cross-env NODE_ENV=production webpack && yarn build-sw",
"build-prod": "yarn build:clean && yarn build-photon && cross-env NODE_ENV=production webpack --progress && yarn build-sw",
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, we need to call build-sw at the end because it depends on the webpack outputs, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right.

@mstange
Copy link
Contributor Author

mstange commented Nov 17, 2025

Hmm, can you tell me more about the reason why this was needed?

Oh, sorry, I forgot to mention: I pulled this out of the esbuild PR (#5589 ) because it's something that can land separately. esbuild doesn't have a workbox plugin, as far as I know.

@mstange
Copy link
Contributor Author

mstange commented Nov 17, 2025

I've tested the deploy preview and everything appears to work offline.

Screenshot 2025-11-17 at 1 57 45 PM

I've also checked the cache storage devtools panel and the cache storage contents look correct. The Status column appears to be completely empty though, I'm not sure why that is, but it currently seems to be that way on the production instance too. I thought I remembered seeing "OK" in some of the cells before but I can't reproduce that at the moment.

sw.js has an importScripts("/service-worker-compat.js") statement both before and after this change.

@canova
Copy link
Member

canova commented Nov 18, 2025

Great, thanks for the context and confirming that it works.

The Status column appears to be completely empty though, I'm not sure why that is, but it currently seems to be that way on the production instance too.

Yeah, I can also see that my production has empty status too. I think it's fine.

LGTM!

@mstange mstange enabled auto-merge November 18, 2025 15:12
@mstange mstange merged commit c07faaf into firefox-devtools:main Nov 18, 2025
12 of 13 checks passed
@canova canova mentioned this pull request Dec 18, 2025
canova added a commit that referenced this pull request Dec 18, 2025
Changes:

[Markus Stange] Use a longer test timeout when debugging with VS code.
(#5679)
[Markus Stange] Move Jest config from package.json to jest.config.js
(#5680)
[Markus Stange] Make binary profile format parsing use Uint8Array
instead of ArrayBuffer (#5678)
[Markus Stange] Use workbox-cli to generate the service worker (#5681)
[Nazım Can Altınova] Migrate from Appveyor to GitHub Actions Windows
runners (#5660)
[Nazım Can Altınova] Remove some unused dependencies (#5696)
[Nazım Can Altınova] Update the document links and sections (#5705)
[Nazım Can Altınova] Clear selected and expanded call node paths on
browser back button if it removes transforms (#5701)
[Nazım Can Altınova] Properly type the Map and Set objects (#5623)
[Valentin Gosu] Add priorityHeader field to network requests (#5707)
[Nazım Can Altınova] Redirect unpublished url loads to the homepage
similar to from-file (#5712)
[Florian Quèze/Nazım Can Altınova] Add an importer for the text format
taken as input by flamegraph.pl. (#5359)
[Florian Quèze] Improve the import of profiles generated from clang
-ftime-trace=file.json (#5714)
[Markus Stange] Move React stuff out of marker schema logic module.
(#5720)

And thanks to our localizers:

en-CA: chutten
en-CA: Paul
es-CL: ravmn
fr: Théo Chevalier
fur: Fabio Tomat
ru: berry
tr: Selim Şumlu
zh-CN: Olvcpr423
zh-CN: wxie
Sign up for free to 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