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

tech story: Update packages to address dependabot security concerns #63

Merged
merged 6 commits into from
Sep 17, 2024

Conversation

coliu-akamai
Copy link
Contributor

@coliu-akamai coliu-akamai commented Sep 12, 2024

Description 📝

Updates (most) packages to their latest version in order to address the dependabot security concerns. Removes package-lock.json since that is old/not necessary with yarn.lock

Major Changes 🔄

  • updated packages to latest version, including react, eslint and eslint related packages
  • updated node version to v18.18.0, so that we could upgrade to @typescript/eslint/-- to v7+ (v8.5.0)
    • I'd tried to update to v20.17.0 like cloud manager but that's leading to some errors when running yarn generate - will look into some more

Testing

  • make sure running yarn generate doesn't result in file differences
  • make sure storybook still works as expected
  • double check that nothing broke in the code

@coliu-akamai coliu-akamai reopened this Sep 13, 2024
@coliu-akamai
Copy link
Contributor Author

Potential updates remaining:
image

  • Avoided updating @types/node bc I couldn't really find a changelog for what v22 entails (but seems like it might correspond to node v22??)
  • Avoided updating eslint to v9 since that results in an unmet peer dependency
  • Updating style-dictionary to 4.1.1 will change all '0rem' to just '0' when running yarn generate if we want to do this
    image
  • seems safe enough to update tokens-studio to 1.2.4?

@jaalah-akamai

"@storybook/react": "^8.3.0",
"@storybook/react-vite": "^8.3.0",
"@storybook/test": "^8.3.0",
"@storybook/theming": "^8.3.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we could only use one set of storybook dependencies between CM & DLS. (and maybe a couple other packages). DLS is here to stay now, so it could potentially server as a sole provide for some deps (I would only consider devDependencies)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yea would be a benefit if we maintain the monorepo approach where we can have one package.json hoisted that's shared across packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👀

Created an internal ticket [M3-8596] for investigating this!

@jaalah-akamai
Copy link
Collaborator

✅ No changes to dist file
✅ Storybook looks good
✅ No breaking changes

@jaalah-akamai
Copy link
Collaborator

@coliu-akamai Thanks for the suggestions above, I'll tackle those in a separate PR.

@coliu-akamai coliu-akamai changed the title fix: Update packages to address dependabot security concerns tech story: Update packages to address dependabot security concerns Sep 17, 2024
@jaalah-akamai jaalah-akamai merged commit b414772 into linode:staging Sep 17, 2024
1 check passed
jaalah-akamai pushed a commit that referenced this pull request Sep 18, 2024
)

* remove package json

* update packages - will need to upgrade node to fully upgrade @typescript-eslint stuff??

* upgrade node to 18.18 + upgrade typescript/eslint dependencies

* update rest of packages that seem safe to update

* update node versions for yml files to match updated package.json? hmm
Sign up for free to 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