fix: cleanup star/unstar by wraithgar · Pull Request #4868 · npm/cli

wraithgar · 2022-05-07T16:12:11Z

It was querying whoami once for every package you starred/unstarred, and
incorrectly trying to determine if you weren't logged in. In fact the
function throws a descriptive message if you're not logged in already.
The whoami check was also racing with the fetch of the packument for
each package you were starring/unstarring meaning you could also get a
random 401 for a private package instead of the 'you need to log in'
message.

unstar was setting an undocumented config item to get the
shared code to unstar. The command already has a name attribute that
tells us what action we are doing so we can just use that.

Finally, the duplicated (and differing) params between the two commands
were consolidated.

It was querying whoami once for every package you starred/unstarred, and incorrectly trying to determine if you weren't logged in. In fact the function throws a descriptive message if you're not logged in already. The whoami check was also racing with the fetch of the packument for each package you were starring/unstarring meaning you could also get a random 401 for a private package instead of the 'you need to log in' message. unstar was setting an undocumented config item to get the shared code to unstar. The command already has a name attribute that tells us what action we are doing so we can just use that. Finally, the duplicated (and differing) params between the two commands were consolidated.

npm-robot · 2022-05-07T16:47:46Z

no statistically significant performance changes detected

timing results

app-large	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	52.222 ±1.05	33.812 ±0.00	36.502 ±23.79	21.459 ±0.64	3.009 ±0.00	3.016 ±0.07	2.475 ±0.20	12.289 ±0.01	2.421 ±0.04	3.466 ±0.00
#4868	55.368 ±0.65	33.046 ±0.07	33.093 ±19.68	21.705 ±1.04	3.013 ±0.00	2.988 ±0.00	2.405 ±0.04	12.266 ±0.01	2.394 ±0.03	3.599 ±0.26

app-medium	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	39.894 ±0.83	25.055 ±0.06	14.454 ±0.14	14.651 ±0.24	2.771 ±0.04	2.795 ±0.05	2.434 ±0.01	8.977 ±0.16	2.274 ±0.00	3.086 ±0.02
#4868	40.108 ±0.92	25.695 ±0.16	13.727 ±0.12	14.645 ±0.28	2.763 ±0.02	2.788 ±0.02	2.489 ±0.03	8.954 ±0.07	2.263 ±0.01	3.240 ±0.14

wraithgar · 2022-05-07T16:12:11Z

It was querying whoami once for every package you starred/unstarred, and
incorrectly trying to determine if you weren't logged in. In fact the
function throws a descriptive message if you're not logged in already.
The whoami check was also racing with the fetch of the packument for
each package you were starring/unstarring meaning you could also get a
random 401 for a private package instead of the 'you need to log in'
message.

unstar was setting an undocumented config item to get the
shared code to unstar. The command already has a name attribute that
tells us what action we are doing so we can just use that.

Finally, the duplicated (and differing) params between the two commands
were consolidated.

It was querying whoami once for every package you starred/unstarred, and incorrectly trying to determine if you weren't logged in. In fact the function throws a descriptive message if you're not logged in already. The whoami check was also racing with the fetch of the packument for each package you were starring/unstarring meaning you could also get a random 401 for a private package instead of the 'you need to log in' message. unstar was setting an undocumented config item to get the shared code to unstar. The command already has a name attribute that tells us what action we are doing so we can just use that. Finally, the duplicated (and differing) params between the two commands were consolidated.

npm-robot · 2022-05-07T16:47:46Z

no statistically significant performance changes detected

timing results

app-large	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	52.222 ±1.05	33.812 ±0.00	36.502 ±23.79	21.459 ±0.64	3.009 ±0.00	3.016 ±0.07	2.475 ±0.20	12.289 ±0.01	2.421 ±0.04	3.466 ±0.00
#4868	55.368 ±0.65	33.046 ±0.07	33.093 ±19.68	21.705 ±1.04	3.013 ±0.00	2.988 ±0.00	2.405 ±0.04	12.266 ±0.01	2.394 ±0.03	3.599 ±0.26

app-medium	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	39.894 ±0.83	25.055 ±0.06	14.454 ±0.14	14.651 ±0.24	2.771 ±0.04	2.795 ±0.05	2.434 ±0.01	8.977 ±0.16	2.274 ±0.00	3.086 ±0.02
#4868	40.108 ±0.92	25.695 ±0.16	13.727 ±0.12	14.645 ±0.28	2.763 ±0.02	2.788 ±0.02	2.489 ±0.03	8.954 ±0.07	2.263 ±0.01	3.240 ±0.14

wraithgar · 2022-05-07T16:12:11Z

It was querying whoami once for every package you starred/unstarred, and
incorrectly trying to determine if you weren't logged in. In fact the
function throws a descriptive message if you're not logged in already.
The whoami check was also racing with the fetch of the packument for
each package you were starring/unstarring meaning you could also get a
random 401 for a private package instead of the 'you need to log in'
message.

unstar was setting an undocumented config item to get the
shared code to unstar. The command already has a name attribute that
tells us what action we are doing so we can just use that.

Finally, the duplicated (and differing) params between the two commands
were consolidated.

It was querying whoami once for every package you starred/unstarred, and incorrectly trying to determine if you weren't logged in. In fact the function throws a descriptive message if you're not logged in already. The whoami check was also racing with the fetch of the packument for each package you were starring/unstarring meaning you could also get a random 401 for a private package instead of the 'you need to log in' message. unstar was setting an undocumented config item to get the shared code to unstar. The command already has a name attribute that tells us what action we are doing so we can just use that. Finally, the duplicated (and differing) params between the two commands were consolidated.

npm-robot · 2022-05-07T16:47:46Z

no statistically significant performance changes detected

timing results

app-large	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	52.222 ±1.05	33.812 ±0.00	36.502 ±23.79	21.459 ±0.64	3.009 ±0.00	3.016 ±0.07	2.475 ±0.20	12.289 ±0.01	2.421 ±0.04	3.466 ±0.00
#4868	55.368 ±0.65	33.046 ±0.07	33.093 ±19.68	21.705 ±1.04	3.013 ±0.00	2.988 ±0.00	2.405 ±0.04	12.266 ±0.01	2.394 ±0.03	3.599 ±0.26

app-medium	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	39.894 ±0.83	25.055 ±0.06	14.454 ±0.14	14.651 ±0.24	2.771 ±0.04	2.795 ±0.05	2.434 ±0.01	8.977 ±0.16	2.274 ±0.00	3.086 ±0.02
#4868	40.108 ±0.92	25.695 ±0.16	13.727 ±0.12	14.645 ±0.28	2.763 ±0.02	2.788 ±0.02	2.489 ±0.03	8.954 ±0.07	2.263 ±0.01	3.240 ±0.14

wraithgar · 2022-05-07T16:12:11Z

It was querying whoami once for every package you starred/unstarred, and
incorrectly trying to determine if you weren't logged in. In fact the
function throws a descriptive message if you're not logged in already.
The whoami check was also racing with the fetch of the packument for
each package you were starring/unstarring meaning you could also get a
random 401 for a private package instead of the 'you need to log in'
message.

unstar was setting an undocumented config item to get the
shared code to unstar. The command already has a name attribute that
tells us what action we are doing so we can just use that.

Finally, the duplicated (and differing) params between the two commands
were consolidated.

It was querying whoami once for every package you starred/unstarred, and incorrectly trying to determine if you weren't logged in. In fact the function throws a descriptive message if you're not logged in already. The whoami check was also racing with the fetch of the packument for each package you were starring/unstarring meaning you could also get a random 401 for a private package instead of the 'you need to log in' message. unstar was setting an undocumented config item to get the shared code to unstar. The command already has a name attribute that tells us what action we are doing so we can just use that. Finally, the duplicated (and differing) params between the two commands were consolidated.

npm-robot · 2022-05-07T16:47:46Z

no statistically significant performance changes detected

timing results

app-large	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	52.222 ±1.05	33.812 ±0.00	36.502 ±23.79	21.459 ±0.64	3.009 ±0.00	3.016 ±0.07	2.475 ±0.20	12.289 ±0.01	2.421 ±0.04	3.466 ±0.00
#4868	55.368 ±0.65	33.046 ±0.07	33.093 ±19.68	21.705 ±1.04	3.013 ±0.00	2.988 ±0.00	2.405 ±0.04	12.266 ±0.01	2.394 ±0.03	3.599 ±0.26

app-medium	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	39.894 ±0.83	25.055 ±0.06	14.454 ±0.14	14.651 ±0.24	2.771 ±0.04	2.795 ±0.05	2.434 ±0.01	8.977 ±0.16	2.274 ±0.00	3.086 ±0.02
#4868	40.108 ±0.92	25.695 ±0.16	13.727 ±0.12	14.645 ±0.28	2.763 ±0.02	2.788 ±0.02	2.489 ±0.03	8.954 ±0.07	2.263 ±0.01	3.240 ±0.14

wraithgar · 2022-05-07T16:12:11Z

It was querying whoami once for every package you starred/unstarred, and
incorrectly trying to determine if you weren't logged in. In fact the
function throws a descriptive message if you're not logged in already.
The whoami check was also racing with the fetch of the packument for
each package you were starring/unstarring meaning you could also get a
random 401 for a private package instead of the 'you need to log in'
message.

unstar was setting an undocumented config item to get the
shared code to unstar. The command already has a name attribute that
tells us what action we are doing so we can just use that.

Finally, the duplicated (and differing) params between the two commands
were consolidated.

It was querying whoami once for every package you starred/unstarred, and incorrectly trying to determine if you weren't logged in. In fact the function throws a descriptive message if you're not logged in already. The whoami check was also racing with the fetch of the packument for each package you were starring/unstarring meaning you could also get a random 401 for a private package instead of the 'you need to log in' message. unstar was setting an undocumented config item to get the shared code to unstar. The command already has a name attribute that tells us what action we are doing so we can just use that. Finally, the duplicated (and differing) params between the two commands were consolidated.

npm-robot · 2022-05-07T16:47:46Z

no statistically significant performance changes detected

timing results

app-large	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	52.222 ±1.05	33.812 ±0.00	36.502 ±23.79	21.459 ±0.64	3.009 ±0.00	3.016 ±0.07	2.475 ±0.20	12.289 ±0.01	2.421 ±0.04	3.466 ±0.00
#4868	55.368 ±0.65	33.046 ±0.07	33.093 ±19.68	21.705 ±1.04	3.013 ±0.00	2.988 ±0.00	2.405 ±0.04	12.266 ±0.01	2.394 ±0.03	3.599 ±0.26

app-medium	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	39.894 ±0.83	25.055 ±0.06	14.454 ±0.14	14.651 ±0.24	2.771 ±0.04	2.795 ±0.05	2.434 ±0.01	8.977 ±0.16	2.274 ±0.00	3.086 ±0.02
#4868	40.108 ±0.92	25.695 ±0.16	13.727 ±0.12	14.645 ±0.28	2.763 ±0.02	2.788 ±0.02	2.489 ±0.03	8.954 ±0.07	2.263 ±0.01	3.240 ±0.14

wraithgar · 2022-05-07T16:12:11Z

It was querying whoami once for every package you starred/unstarred, and
incorrectly trying to determine if you weren't logged in. In fact the
function throws a descriptive message if you're not logged in already.
The whoami check was also racing with the fetch of the packument for
each package you were starring/unstarring meaning you could also get a
random 401 for a private package instead of the 'you need to log in'
message.

unstar was setting an undocumented config item to get the
shared code to unstar. The command already has a name attribute that
tells us what action we are doing so we can just use that.

Finally, the duplicated (and differing) params between the two commands
were consolidated.

It was querying whoami once for every package you starred/unstarred, and incorrectly trying to determine if you weren't logged in. In fact the function throws a descriptive message if you're not logged in already. The whoami check was also racing with the fetch of the packument for each package you were starring/unstarring meaning you could also get a random 401 for a private package instead of the 'you need to log in' message. unstar was setting an undocumented config item to get the shared code to unstar. The command already has a name attribute that tells us what action we are doing so we can just use that. Finally, the duplicated (and differing) params between the two commands were consolidated.

npm-robot · 2022-05-07T16:47:46Z

no statistically significant performance changes detected

timing results

app-large	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	52.222 ±1.05	33.812 ±0.00	36.502 ±23.79	21.459 ±0.64	3.009 ±0.00	3.016 ±0.07	2.475 ±0.20	12.289 ±0.01	2.421 ±0.04	3.466 ±0.00
#4868	55.368 ±0.65	33.046 ±0.07	33.093 ±19.68	21.705 ±1.04	3.013 ±0.00	2.988 ±0.00	2.405 ±0.04	12.266 ±0.01	2.394 ±0.03	3.599 ±0.26

app-medium	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	39.894 ±0.83	25.055 ±0.06	14.454 ±0.14	14.651 ±0.24	2.771 ±0.04	2.795 ±0.05	2.434 ±0.01	8.977 ±0.16	2.274 ±0.00	3.086 ±0.02
#4868	40.108 ±0.92	25.695 ±0.16	13.727 ±0.12	14.645 ±0.28	2.763 ±0.02	2.788 ±0.02	2.489 ±0.03	8.954 ±0.07	2.263 ±0.01	3.240 ±0.14

wraithgar · 2022-05-07T16:12:11Z

It was querying whoami once for every package you starred/unstarred, and
incorrectly trying to determine if you weren't logged in. In fact the
function throws a descriptive message if you're not logged in already.
The whoami check was also racing with the fetch of the packument for
each package you were starring/unstarring meaning you could also get a
random 401 for a private package instead of the 'you need to log in'
message.

unstar was setting an undocumented config item to get the
shared code to unstar. The command already has a name attribute that
tells us what action we are doing so we can just use that.

Finally, the duplicated (and differing) params between the two commands
were consolidated.

It was querying whoami once for every package you starred/unstarred, and incorrectly trying to determine if you weren't logged in. In fact the function throws a descriptive message if you're not logged in already. The whoami check was also racing with the fetch of the packument for each package you were starring/unstarring meaning you could also get a random 401 for a private package instead of the 'you need to log in' message. unstar was setting an undocumented config item to get the shared code to unstar. The command already has a name attribute that tells us what action we are doing so we can just use that. Finally, the duplicated (and differing) params between the two commands were consolidated.

npm-robot · 2022-05-07T16:47:46Z

no statistically significant performance changes detected

timing results

app-large	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	52.222 ±1.05	33.812 ±0.00	36.502 ±23.79	21.459 ±0.64	3.009 ±0.00	3.016 ±0.07	2.475 ±0.20	12.289 ±0.01	2.421 ±0.04	3.466 ±0.00
#4868	55.368 ±0.65	33.046 ±0.07	33.093 ±19.68	21.705 ±1.04	3.013 ±0.00	2.988 ±0.00	2.405 ±0.04	12.266 ±0.01	2.394 ±0.03	3.599 ±0.26

app-medium	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	39.894 ±0.83	25.055 ±0.06	14.454 ±0.14	14.651 ±0.24	2.771 ±0.04	2.795 ±0.05	2.434 ±0.01	8.977 ±0.16	2.274 ±0.00	3.086 ±0.02
#4868	40.108 ±0.92	25.695 ±0.16	13.727 ±0.12	14.645 ±0.28	2.763 ±0.02	2.788 ±0.02	2.489 ±0.03	8.954 ±0.07	2.263 ±0.01	3.240 ±0.14

wraithgar · 2022-05-07T16:12:11Z

It was querying whoami once for every package you starred/unstarred, and
incorrectly trying to determine if you weren't logged in. In fact the
function throws a descriptive message if you're not logged in already.
The whoami check was also racing with the fetch of the packument for
each package you were starring/unstarring meaning you could also get a
random 401 for a private package instead of the 'you need to log in'
message.

unstar was setting an undocumented config item to get the
shared code to unstar. The command already has a name attribute that
tells us what action we are doing so we can just use that.

Finally, the duplicated (and differing) params between the two commands
were consolidated.

It was querying whoami once for every package you starred/unstarred, and incorrectly trying to determine if you weren't logged in. In fact the function throws a descriptive message if you're not logged in already. The whoami check was also racing with the fetch of the packument for each package you were starring/unstarring meaning you could also get a random 401 for a private package instead of the 'you need to log in' message. unstar was setting an undocumented config item to get the shared code to unstar. The command already has a name attribute that tells us what action we are doing so we can just use that. Finally, the duplicated (and differing) params between the two commands were consolidated.

npm-robot · 2022-05-07T16:47:46Z

no statistically significant performance changes detected

timing results

app-large	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	52.222 ±1.05	33.812 ±0.00	36.502 ±23.79	21.459 ±0.64	3.009 ±0.00	3.016 ±0.07	2.475 ±0.20	12.289 ±0.01	2.421 ±0.04	3.466 ±0.00
#4868	55.368 ±0.65	33.046 ±0.07	33.093 ±19.68	21.705 ±1.04	3.013 ±0.00	2.988 ±0.00	2.405 ±0.04	12.266 ±0.01	2.394 ±0.03	3.599 ±0.26

app-medium	clean	lock-only	cache-only	cache-only peer-deps	modules-only	no-lock	no-cache	no-modules	no-clean	no-clean audit
npm@8	39.894 ±0.83	25.055 ±0.06	14.454 ±0.14	14.651 ±0.24	2.771 ±0.04	2.795 ±0.05	2.434 ±0.01	8.977 ±0.16	2.274 ±0.00	3.086 ±0.02
#4868	40.108 ±0.92	25.695 ±0.16	13.727 ±0.12	14.645 ±0.28	2.763 ±0.02	2.788 ±0.02	2.489 ±0.03	8.954 ±0.07	2.263 ±0.01	3.240 ±0.14

wraithgar requested a review from a team as a code owner May 7, 2022 16:12

wraithgar force-pushed the gar/star-cleanup branch 2 times, most recently from 0b2a933 to 146b564Compare May 7, 2022 16:15

wraithgar force-pushed the gar/star-cleanup branch from 146b564 to a97bf8fCompare May 7, 2022 16:22

ruyadorno approved these changes May 9, 2022
View reviewed changes

wraithgar merged commit 38cf29a into latestMay 9, 2022

wraithgar deleted the gar/star-cleanup branch May 9, 2022 14:34

wraithgar mentioned this pull request May 11, 2022
Release/v8.10.0 #4884
Merged

npm-cli-bot mentioned this pull request May 11, 2022
deps: upgrade npm to 8.10.0 nodejs/node#43061
Closed

snyk-ioBot mentioned this pull request Oct 22, 2024
[Snyk] Fix for 17 vulnerabilities Graybar-codespace/cli-3#115
Open