Skip to content

Conversation

@zchsh
Copy link
Contributor

@zchsh zchsh commented Mar 9, 2021

🔍 Preview Link


This PR fixes an issue where package-locks did not correctly reflect monorepo interdependencies.

These changes are just the result of a fresh clone and npm i && npm run bootstrap. I think the reason these inconsistencies can occur is that when deps are added to individual packages, lerna doesn't go through the bootstrap process.

Makes me wonder whether each package should have cd ../../ && lerna bootstrap as a postinstall script? 🤔

Release Information

Please select one (required)

  • Major (This PR introduces a breaking (incompatible API) change)
  • Minor (This PR adds functionality but it backwards-compatible)
  • Patch (This PR adds a backwards-compatible bug fix)
Release Notes (required) Fixes an issue where package-locks did not correctly reflect monorepo interdependencies.

PR Checklist 🚀

Items in this checklist may not may not apply to your PR, but please consider each item carefully.

  • Add Asana and Preview links above.
  • Conduct thorough self-review.
  • Add or update tests as appropriate.
  • Write a useful description (above) to give reviewers appropriate context.
  • Add release notes to the appropriate section (above).
  • Conduct reasonable cross browser testing for both compatibility and responsive behavior (We have a Cross Browser Testing account for this, if you don't have access, just ask!).
  • Conduct reasonable accessibility review (use the WAS as a guide or an axe browser plugin until we establish more formal checks).
  • Identify (in the description above) and document (add Asana tasks on this board) any technical debt that you're aware of, but are not addressing as part of this PR.

@vercel
Copy link

vercel bot commented Mar 9, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/hashicorp/react-components/32USPGk3sMPpmjP5iFsP8qbB3h8U
✅ Preview: https://react-components-git-zsfix-package-locks-hashicorp.vercel.app

@zchsh zchsh marked this pull request as ready for review March 9, 2021 16:07
@zchsh zchsh requested a review from brkalow March 9, 2021 16:07
@zchsh
Copy link
Contributor Author

zchsh commented Mar 9, 2021

@brkalow for context this is a pretty chore-y PR, I'd missed some subtle changes to package-locks in #154.

Apart from a second set of eyes, would also love your take on possible longer-term solutions for these types of issues... eg does apostinstall: "cd ../../ && npm run bootstrap script make any sense? 🤔 (I have yet to try it, will go do that now 🏃 )

EDIT: nvm the postinstall thing seems like a bad idea, then when running npm run bootstrap from the root it gets stuck in an infinite loop 😬

@brkalow
Copy link
Contributor

brkalow commented Mar 9, 2021

I bet we can add npm run bootstrap to our CI process, which should cause npm ci to fail if the package-lock has local changes.

@zchsh
Copy link
Contributor Author

zchsh commented Mar 9, 2021

@brkalow yeah that's a great call! Any objections to merging this PR in for now? (Just looking to publish docs-page and -sidenav from #154 )

Copy link
Contributor

@brkalow brkalow left a comment

Choose a reason for hiding this comment

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

Let's do it!

@zchsh zchsh merged commit c95a212 into main Mar 9, 2021
@zchsh zchsh deleted the zs.fix-package-locks branch March 9, 2021 16:51
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.

2 participants