feat(types): add Packages to replace types from @manypkg/get-packages#1872
feat(types): add Packages to replace types from @manypkg/get-packages#1872Andarist merged 5 commits intochangesets:nextfrom
Packages to replace types from @manypkg/get-packages#1872Conversation
🦋 Changeset detectedLatest commit: 40f95d3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| changesets: string[]; | ||
| }; | ||
|
|
||
| export interface ChangesetsPackage { |
There was a problem hiding this comment.
given we are in the scope of Changesets already, I'd prefer to remove the Changesets prefix from those types
There was a problem hiding this comment.
I prefixed the name with Changeset to avoid clashes with the ones coming from manypkg.
I can rename them now.
| const packagesByName: { [key: string]: Package } = { | ||
| [packages.root.packageJson.name]: packages.root, | ||
| const packagesByName: { [key: string]: ChangesetsPackage } = { | ||
| [packages.root!.packageJson.name]: packages.root!, |
There was a problem hiding this comment.
isn't root required in the new type?
There was a problem hiding this comment.
Leftover, the not null assertion shouldn't be needed.
Maybe in the future I would consider turning on the no-non-null-assertion rule that reports similar situations.
EDIT: Maybe in the future I would consider turning on the no-unnecessary-type-assertion rule that reports similar situations.
There was a problem hiding this comment.
|
|
||
| export default function getDependencyGraph( | ||
| packages: Packages, | ||
| packages: Pick<ChangesetsPackages, "packages" | "root">, |
There was a problem hiding this comment.
q: why do we use Pick here now?
There was a problem hiding this comment.
I used a Pick since this package only accesses those two properties, so I thought it would be clearer to make that explicit.
| await git.commit("first commit", tempDir); | ||
|
|
||
| try { | ||
| const { root, packages, tool } = await getPackages(tempDir); |
There was a problem hiding this comment.
q: so now tool is just a string so we wrap it in { type: tool } to conform to thy manypkg's v2 shape, right? once we migrate to it this value will change, we'll be able to just pass in tool directly because it will be an object with a .type property. On our end, in Changesets, we'll only require the .type to exist though
There was a problem hiding this comment.
Exactly.
The idea is to use an object so it can be extended with additional fields in the future if needed.
For now, the main benefit is that it keeps the type compatible with the one from manypkg@v2.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #1872 +/- ##
=======================================
Coverage 82.50% 82.50%
=======================================
Files 53 53
Lines 2383 2384 +1
Branches 697 697
=======================================
+ Hits 1966 1967 +1
Misses 375 375
Partials 42 42 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ChangesetsPackagesPackages to replace types from @manypkg/get-packages
@manypkg/get-packagestov2#1795This PR introduces
ChangesetsPackagestypes to reduce the number of package relying on@manypkg/get-packages