Skip to content

fix(refactor): use output buffer and error for more commands - #7513

Merged
lukekarrys merged 5 commits into
latestfrom
lk/publish-output-buffer
May 15, 2024
Merged

fix(refactor): use output buffer and error for more commands#7513
lukekarrys merged 5 commits into
latestfrom
lk/publish-output-buffer

Conversation

@lukekarrys

@lukekarryslukekarrys commented May 12, 2024

Copy link
Copy Markdown
Contributor

This changes a bunch of commands to use the new output.buffer capabilities from proc-log as well as the outputError helper from utils/output-error.js.

This also adds a few comments about future display related breaking changes that npm should make.

There is some new behavior around run-script and how it outputs errors. It now displays the error for each workspace similar to how that error would get displayed when the process exits.

Here are some screenshots of the new run-script output to show with coloring. These are all represented in snapshots as well both those can be hard to decipher with ansi codes. The screenshots are run via set -x; npm $@; npmlocal $@ so the output preceded with + npm is just to show the command being run.

Running a non-existent command from multiple workspaces
Screenshot 2024-05-14 at 9 50 06 AM

Running a script that errors from multiple workspaces
Screenshot 2024-05-14 at 9 57 24 AM

Listing scripts

The only change here is I removed the newline between lifecycle scripts and run scripts. I find the indent shows the hierarchy of information and the extra newline made it difficult to quickly scan multiple workspaces which are also separated by newlines.

Screenshot 2024-05-14 at 10 14 13 AM

@npm-cli-bot

npm-cli-bot commented May 12, 2024

Copy link
Copy Markdown
Collaborator

no statistically significant performance changes detected

timing results
app-largecleanlock-onlycache-onlymodules-onlyno-lockno-cacheno-modulesno-cleanshow-versionrun-scriptcache-only
peer-deps
no-clean
audit
npm@latest37.754 ±0.7110.985 ±0.0012.062 ±0.051.588 ±0.001.587 ±0.011.307 ±0.008.415 ±0.081.319 ±0.010.143 ±0.000.170 ±0.0013.827 ±0.153.965 ±2.58
#751335.637 ±0.8010.876 ±0.0012.096 ±0.211.585 ±0.011.564 ±0.011.291 ±0.018.687 ±0.391.325 ±0.000.142 ±0.000.170 ±0.0015.174 ±0.192.163 ±0.01
app-mediumcleanlock-onlycache-onlymodules-onlyno-lockno-cacheno-modulesno-cleanshow-versionrun-scriptcache-only
peer-deps
no-clean
audit
npm@latest28.833 ±0.188.128 ±0.089.090 ±0.001.554 ±0.011.530 ±0.001.454 ±0.005.951 ±0.031.358 ±0.020.143 ±0.000.170 ±0.009.642 ±0.023.307 ±1.87
#751327.231 ±3.688.240 ±0.049.029 ±0.021.534 ±0.011.540 ±0.031.518 ±0.025.882 ±0.011.335 ±0.010.142 ±0.000.170 ±0.009.944 ±0.122.049 ±0.09

@lukekarrys
lukekarrysforce-pushed the lk/publish-output-buffer branch 3 times, most recently from deb96a5 to 71d5e0aCompareMay 13, 2024 02:59
@lukekarrys
lukekarrys changed the base branch from latest to lk/errors-and-jsonMay 13, 2024 03:00
@lukekarrys
lukekarrysforce-pushed the lk/publish-output-buffer branch from 71d5e0a to c462312CompareMay 13, 2024 04:28
@lukekarrys
lukekarrysforce-pushed the lk/publish-output-buffer branch 2 times, most recently from 872be60 to 2d0eb9dCompareMay 13, 2024 06:31
@lukekarryslukekarrys changed the title fix(refactor): use output.buffer for easier json output in publishfix(refactor): use output.buffer for json outputMay 13, 2024
@lukekarrys
lukekarrysforce-pushed the lk/publish-output-buffer branch from 2d0eb9d to 5363c9fCompareMay 13, 2024 07:01
Base automatically changed from lk/errors-and-json to latestMay 13, 2024 17:24
@lukekarrys
lukekarrysforce-pushed the lk/publish-output-buffer branch 9 times, most recently from 75a3c10 to 3a62c37CompareMay 14, 2024 17:02
@lukekarryslukekarrys changed the title fix(refactor): use output.buffer for json outputfix(refactor): use output buffer and error for more commandsMay 14, 2024
This changes a bunch of commands to use the new `output.buffer` capabilities from `proc-log` as well as the `outputError` helper from `utils/output-error.js`.
There is some new behavior around `run-script` and how it outputs errors. It now displays the error for each workspace similar to how that error would get displayed when the process exits.
@lukekarrys
lukekarrysforce-pushed the lk/publish-output-buffer branch from 3a62c37 to 1d64884CompareMay 14, 2024 17:10
@lukekarrys
lukekarrys marked this pull request as ready for review May 14, 2024 17:12
@lukekarrys
lukekarrys requested a review from a team as a code ownerMay 14, 2024 17:12
Comment threadlib/commands/run-script.js Outdated
Comment threadlib/commands/run-script.js Outdated
Comment threadlib/utils/display.js Outdated
@lukekarrys
lukekarrysforce-pushed the lk/publish-output-buffer branch from 80e6de4 to 9da2d27CompareMay 15, 2024 18:53
@lukekarrys
lukekarrysforce-pushed the lk/publish-output-buffer branch from 9da2d27 to 11a49f9CompareMay 15, 2024 19:11
@lukekarrys
lukekarrys requested a review from wraithgarMay 15, 2024 19:20
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.

3 participants

@lukekarrys@npm-cli-bot@wraithgar