Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: Fix "Invalid version null" issues by improved version detection #22642

Merged
merged 30 commits into from
Jun 15, 2023

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented May 19, 2023

Closes #22345

What I did

  • Removed bower support
  • Implemented methods to read the version of an installed package
  • Fixed some places, which weren't pnpm sensitive
  • Removed Angular 12 automigrations
  • Fixed upgrade and automigrate commands in nx projects
  • Fixed setting eslint-plugin in eslintPlugin automigrate
  • Fixed Storybook upgrade/init commands in yarn workspace environments
  • Fixed a bug where nextjs wasn't working properly in pnp environments
  • Fixed detection of pnp in workspaces when Storybook init was called in a subdirectory
  • Fixed @storybook/addon-docs not automatically installed when compodoc was in use

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@valentinpalkovic valentinpalkovic added the maintenance User-facing maintenance tasks label May 19, 2023
@valentinpalkovic valentinpalkovic self-assigned this May 19, 2023
@socket-security
Copy link

socket-security bot commented May 19, 2023

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Issue Package Version Note Source
Network access @yarnpkg/libzip 2.3.0 code/lib/cli/package.json via @yarnpkg/fslib@2.10.3

Next steps

What is network access?

This module accesses the network.

Packages should remove all network access that isn't functionally unnecessary. Consumers should audit network access to ensure legitimate use.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

  • @SocketSecurity ignore @yarnpkg/libzip@2.3.0

@valentinpalkovic valentinpalkovic force-pushed the valentin/invalid-version-null branch from 390a920 to 9f43b06 Compare May 19, 2023 13:31
@valentinpalkovic valentinpalkovic changed the title Valentin/invalid version null Fix "Invalid version null" bug May 19, 2023
@valentinpalkovic valentinpalkovic marked this pull request as ready for review May 19, 2023 13:34
@valentinpalkovic valentinpalkovic force-pushed the valentin/invalid-version-null branch from 9f43b06 to 55ec140 Compare May 19, 2023 13:54
@valentinpalkovic valentinpalkovic force-pushed the valentin/invalid-version-null branch from 0dd0939 to 6c6f3af Compare May 22, 2023 15:36
@ndelangen
Copy link
Member

ndelangen commented May 23, 2023

Can this be merged @valentinpalkovic ?

@yannbf
Copy link
Member

yannbf commented May 23, 2023

Can this be merged @valentinpalkovic ?

No, I'm working on it!

Copy link
Member

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

Let me start by saying that this is super awesome ❤️

Unfortunately it seems like the pnp solutions do not work?

Yarn2

• Detecting project typeError while fetching package version in Yarn PnP mode: Error: Your application tried to access nx, but it isn't declared in your dependencies; this makes the require call ambiguous and unsound.

Required package: nx
Required by: /Users/yannbraga/experiments/delete/react-ts

    at makeError (/Users/yannbraga/experiments/delete/react-ts/.pnp.cjs:21064:34)
    at resolveToUnqualified (/Users/yannbraga/experiments/delete/react-ts/.pnp.cjs:22781:21)
    at Object.resolveToUnqualified (/Users/yannbraga/experiments/delete/react-ts/.pnp.cjs:22973:26)
    at Yarn2Proxy.getPackageVersion (/Users/yannbraga/open-source/storybook/code/lib/cli/dist/generate.js:1751:43)
    at isNxProject (/Users/yannbraga/open-source/storybook/code/lib/cli/dist/generate.js:535:42)

And similar errors happen when trying to resolve prettier, @babel/plugin-transform-typescript, eslint-plugin-storybook in the same execution

PNPM

Detecting project typeError while fetching package version in Yarn PnP mode: Error: Your application tried to access nx, but it isn't declared in your dependencies; this makes the require call ambiguous and unsound.

Required package: nx (via "nx")
Required by: /Users/yannbraga/experiments/delete/react-ts

    at internalTools_makeError (/Users/yannbraga/experiments/delete/react-ts/.pnp.cjs:13925:34)
    at resolveToUnqualified (/Users/yannbraga/experiments/delete/react-ts/.pnp.cjs:14884:23)
    at Object.resolveToUnqualified (/Users/yannbraga/experiments/delete/react-ts/.pnp.cjs:15051:26)
    at PNPMProxy.getPackageVersion (/Users/yannbraga/open-source/storybook/code/lib/cli/dist/generate.js:1492:43)
    at isNxProject (/Users/yannbraga/open-source/storybook/code/lib/cli/dist/generate.js:535:42)
    at detect (/Users/yannbraga/open-source/storybook/code/lib/cli/dist/generate.js:673:13)

It also feels a little weird having tons of errors but the storybook init command continues “normally” as these errors are console.error messages

Can you take a look at that?

@valentinpalkovic valentinpalkovic force-pushed the valentin/invalid-version-null branch from 06c8987 to f01a652 Compare May 30, 2023 13:40
@shilman shilman changed the title Fix "Invalid version null" bug CLI: Fix "Invalid version null" bug May 31, 2023
@shilman shilman added bug cli patch:yes Bugfix & documentation PR that need to be picked to main branch and removed maintenance User-facing maintenance tasks labels May 31, 2023
@shilman shilman changed the title CLI: Fix "Invalid version null" bug CLI: Fix "Invalid version null" May 31, 2023
@valentinpalkovic valentinpalkovic force-pushed the valentin/invalid-version-null branch from 49816c5 to 8f557f7 Compare June 6, 2023 08:19
@valentinpalkovic valentinpalkovic requested a review from a team as a code owner June 6, 2023 08:19
@socket-security
Copy link

New and updated dependency changes detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives1 Size Publisher
@yarnpkg/libzip 🆕 2.3.0 network, filesystem +1 785 kB arcanis
@yarnpkg/fslib 🆕 2.10.3 filesystem +2 1.06 MB arcanis

Footnotes

  1. https://docs.socket.dev

@yannbf
Copy link
Member

yannbf commented Jun 12, 2023

Unfortunately there is something going on where the output of automigration affects the other and the user gets stuck. I have to investigate what's causing it.

image

@yannbf yannbf added the ci:daily Run the CI jobs that normally run in the daily job. label Jun 15, 2023
@yannbf
Copy link
Member

yannbf commented Jun 15, 2023

Time to merge this!!

@yannbf yannbf changed the title CLI: Fix "Invalid version null" CLI: Improve version detection of packages, fixing various "Invalid version null" issues Jun 15, 2023
@yannbf yannbf removed the ci:daily Run the CI jobs that normally run in the daily job. label Jun 15, 2023
@yannbf yannbf force-pushed the valentin/invalid-version-null branch from fa7a977 to 2c3be33 Compare June 15, 2023 16:49
@shilman shilman changed the title CLI: Improve version detection of packages, fixing various "Invalid version null" issues CLI: Fix "Invalid version null" issues by improved version detection Jun 15, 2023
@yannbf yannbf merged commit 88ea97b into next Jun 15, 2023
@yannbf yannbf deleted the valentin/invalid-version-null branch June 15, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

storybook init: Invalid Version: null
4 participants