Skip to content

fix(core): run plugins sequentially#1145

Merged
matejchalk merged 1 commit into
mainfrom
sequential-plugins
Nov 14, 2025
Merged

fix(core): run plugins sequentially#1145
matejchalk merged 1 commit into
mainfrom
sequential-plugins

Conversation

@matejchalk
Copy link
Copy Markdown
Collaborator

This fix should prevent problems with concurrent spinners, such as in #1144.

Plugins are intended to be executed sequentially, but it appears they were (unintentionally?) made concurrent in 7a592eb as part of the large PR #811.

I intend to make more changes to the plugin execution (log groups, etc.), but for now this is just a minimal fix.

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Nov 14, 2025

View your CI Pipeline Execution ↗ for commit c0e7f04

Command Status Duration Result
nx code-pushup --nx-bail -- compare ✅ Succeeded 54s View ↗
nx code-pushup --nx-bail -- ✅ Succeeded 1m 10s View ↗
nx code-pushup --nx-bail -- print-config --outp... ✅ Succeeded 4m 14s View ↗

☁️ Nx Cloud last updated this comment at 2025-11-14 11:30:20 UTC

@nx-cloud
Copy link
Copy Markdown

nx-cloud Bot commented Nov 14, 2025

View your CI Pipeline Execution ↗ for commit c0e7f04


☁️ Nx Cloud last updated this comment at 2025-11-14 11:14:59 UTC

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Nov 14, 2025

Open in StackBlitz

@code-pushup/ci

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/ci@1145

@code-pushup/cli

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/cli@1145

@code-pushup/core

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/core@1145

@code-pushup/create-cli

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/create-cli@1145

@code-pushup/models

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/models@1145

@code-pushup/nx-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/nx-plugin@1145

@code-pushup/axe-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/axe-plugin@1145

@code-pushup/coverage-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/coverage-plugin@1145

@code-pushup/eslint-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/eslint-plugin@1145

@code-pushup/js-packages-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/js-packages-plugin@1145

@code-pushup/jsdocs-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/jsdocs-plugin@1145

@code-pushup/lighthouse-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/lighthouse-plugin@1145

@code-pushup/typescript-plugin

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/typescript-plugin@1145

@code-pushup/utils

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/utils@1145

@code-pushup/models-transformers

npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/models-transformers@1145

commit: c0e7f04

@matejchalk matejchalk added the 🐛 bug something isn't working label Nov 14, 2025
@github-actions
Copy link
Copy Markdown
Contributor

Code PushUp

😟 Code PushUp report has regressed – compared current commit 65d231a with previous commit c90eea2.

🕵️ See full comparison in Code PushUp portal 🔍

🏷️ Categories

🏷️ Category ⭐ Previous score ⭐ Current score 🔄 Score change
Performance 🔴 35 🔴 33 ↓ −2.4
Code coverage 🟡 88 🟡 88
Security 🟡 56 🟡 56
Updates 🟡 75 🟡 75
Accessibility 🟢 92 🟢 92
Best Practices 🟢 100 🟢 100
SEO 🟡 61 🟡 61
Type Safety 🟢 100 🟢 100
Bug prevention 🟢 100 🟢 100
Miscellaneous 🟢 100 🟢 100
Code style 🟢 100 🟢 100
Documentation 🔴 35 🔴 35
👎 1 group regressed, 👎 4 audits regressed, 15 audits changed without impacting score

🗃️ Groups

🔌 Plugin 🗃️ Group ⭐ Previous score ⭐ Current score 🔄 Score change
Lighthouse Performance 🔴 35 🔴 33 ↓ −2.4

22 other groups are unchanged.

🛡️ Audits

🔌 Plugin 🛡️ Audit 📏 Previous value 📏 Current value 🔄 Value change
Lighthouse Speed Index 🟥 6.6 s 🟥 7.5 s ↑ +15.1 %
Lighthouse First Contentful Paint 🟨 3.0 s 🟥 3.3 s ↑ +9.4 %
Lighthouse Total Blocking Time 🟥 2,430 ms 🟥 2,680 ms ↑ +10.5 %
Lighthouse Time to Interactive 🟥 13.3 s 🟥 13.4 s ↑ +0.8 %
Lighthouse Avoids enormous network payloads 🟩 Total size was 2,034 KiB 🟩 Total size was 2,007 KiB ↓ −1.3 %
Lighthouse Uses efficient cache policy on static assets 🟨 30 resources found 🟨 30 resources found ↑ +0.1 %
Lighthouse Minimizes main-thread work 🟥 13.3 s 🟥 13.8 s ↑ +3.3 %
Lighthouse Server Backend Latencies 🟩 1,230 ms 🟩 890 ms ↓ −28.2 %
Lighthouse Max Potential First Input Delay 🟥 1,060 ms 🟥 1,390 ms ↑ +31.3 %
Lighthouse Reduce unused JavaScript 🟥 Potential savings of 183 KiB 🟥 Potential savings of 160 KiB ↓ −23.5 %
Lighthouse Largest Contentful Paint 🟥 11.3 s 🟥 11.4 s ↑ +1.3 %
Lighthouse Metrics 🟩 100% 🟩 100% ↑ +0.8 %
Lighthouse Initial server response time was short 🟩 Root document took 560 ms 🟩 Root document took 470 ms ↓ −16.2 %
Lighthouse JavaScript execution time 🟥 4.9 s 🟥 4.9 s ↓ −1.1 %
Lighthouse Reduce unused CSS 🟥 Potential savings of 105 KiB 🟥 Potential savings of 105 KiB ↓ −8.2 %
Lighthouse Remove duplicate modules in JavaScript bundles 🟥 Potential savings of 102 KiB 🟥 Potential savings of 90 KiB ↓ −8.2 %
Lighthouse Network Round Trip Times 🟩 50 ms 🟩 60 ms ↑ +21 %
Code coverage Branch coverage 🟨 86.7 % 🟨 86.6 % ↓ −0.1 %
Code coverage Line coverage 🟨 84.5 % 🟨 84.5 % ↑ +0.1 %

659 other audits are unchanged.

@matejchalk matejchalk marked this pull request as ready for review November 14, 2025 11:21
@vmasek
Copy link
Copy Markdown
Collaborator

vmasek commented Nov 14, 2025

Is it just "spinners" problem or is there a deeper issue to run in parallel? I honestly thought we mean to run them in parallel.

@matejchalk
Copy link
Copy Markdown
Collaborator Author

Is it just "spinners" problem or is there a deeper issue to run in parallel? I honestly thought we mean to run them in parallel.

As I recall, the initial reason was that concurrent execution would impact the results of plugins that measure performance (e.g., Lighthouse).

However, at this moment in time, a bigger concern for me is that logging and handling errors in concurrent mode would be way more complex. To do it well, we'd probably have to build something like Nx's new Terminal UI, and I don't think it's worth investing our resources into that.

I also couldn't find any well-established packages that support concurrent spinners. Despite its single-spinner limitation, I've had very good experiences with ora (as well as other packages from this author), whereas all the "multi-spinner" packages I could find (listr, spinnies, multispinner) were too niche (all 3 combined have 10x fewer GitHub stars than ora), and, in the case of listr, also too much of an opinionated framework.

Copy link
Copy Markdown
Collaborator

@hanna-skryl hanna-skryl left a comment

Choose a reason for hiding this comment

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

I tested this change, but the concurrent spinner error still occurs:

  1. Applied sequential execution in an external repository
  2. Modified source code and verified changes took effect
  3. Error still thrown during Axe plugin execution

The spinner conflict appears to be caused by something other than parallel plugin execution.

@matejchalk
Copy link
Copy Markdown
Collaborator Author

I tested this change, but the concurrent spinner error still occurs:

  1. Applied sequential execution in an external repository
  2. Modified source code and verified changes took effect
  3. Error still thrown during Axe plugin execution

The spinner conflict appears to be caused by something other than parallel plugin execution.

Weird, I'm not sure why that would be. Concurrent spinners could be caused by Promise.all/Promise.allSettled or spinner nesting. Running another spinner after the previous one resolved should work fine. I added some extra tests to the logger to check this, just in case (#1147).

The only spinner usage is in executeProcess calls at the moment, so I'm having a hard time figuring out how there can be more than 1 running. The only executeProcess I can find in axe-plugin is the playwright install command that runs for each URL, but in your async reduce you await the previous value, so I don't see any possible conflict there. 😕

If you'd like me to help debug the problem, I'll probably need to reproduce it. Do you think you could share some minimal reproduction?

The Axe plugin is working in this repo, so I'd start by trying to figure out what's different about the configuration in the repo where you're seeing the error (what other plugins are used, etc.).

How are you applying the changes from this PR, BTW? Editing the downloaded .js files in node_modules/@code-pushup/core? You could also try installing via npm i https://pkg.pr.new/code-pushup/cli/@code-pushup/axe-plugin@1145.

@hanna-skryl