Conversation
Socket Security Pull Request ReportDependency issues detected. If you merge this pull request, you will not be alerted to the instances of these issues again. 📜 Install scriptsInstall scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts. Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead.
😵💫 Bin script confusionThis package has multiple bin scripts with the same name. This can cause non-deterministic behavior when installing or could be a sign of a supply chain attack Consider removing one of the conflicting packages. Packages should only export bin scripts with their name
Pull request report summary
Bot CommandsTo ignore an alert, reply with a comment starting with
Powered by socket.dev |
a7f7286 to
56a2f05
Compare
georgewrmarshall
left a comment
There was a problem hiding this comment.
Left a comment regarding what we talked about in our extension sync
| export default { | ||
| title: 'Components/App/AccountListItem', | ||
| id: __filename, | ||
| id: 'ui-components-app-account-list-item-account-list-item', |
There was a problem hiding this comment.
Could we just get rid of the id field altogether? From my understanding it does 2 things
- Creates the file path URL schema
- Generates this section of links for each PR build 👇
🤔 I'm not sure it adds a huge amount of value because it seems to include stories that have no relevance to the PR https://github.com/MetaMask/metamask-extension/pull/16931/files.
Plus there is the other link to the storybook build so we could just use that for visual testing?
Pros
- Engineers don't have to fill out another field when creating a story
Cons
- Lose the ability to link to effected stories from the PR
- We would have to update all stories that link to other stories and think about externally links stories from Figma
So would have to update the links in Figma and in our MDX docs
There was a problem hiding this comment.
Yup, we can get rid of the id field altogether. @georgewrmarshall this is the current plan
- We will delete the
idfield from all the stories.js files and won't be relying onfilename/idanymore. - Currently URLs are generated based on the existing ids but when the
idswill be gone, new URLs will be generated based on the title of each story. - In the CI, we can get the URL from stories.json.
- The only con you mentioned will be to update the New URLs in the docs (README.mdx) files.
56a2f05 to
101837e
Compare
* updated colors for typography * updated valid colors
* updated test in picker-network * fixed import order in README * fixed global import path * fixed formatting
Co-authored-by: ryanml <ryanlanese@gmail.com>
…der after account recovery (#17088)
…ox Component (#17085) * added padding/margin inline-start and inline-end support * updated story for margin
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
Closing it with reference to #17092 |
Fixes #15760
test-storybookdependencyTODOS:
Unexpected component id