-
Notifications
You must be signed in to change notification settings - Fork 14
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
chore: extract common declarations #16
Conversation
@@ -11,6 +11,8 @@ export interface PackageJson { | |||
* these help npm searches find your project | |||
*/ | |||
keywords?: string[]; | |||
homepage?: string; | |||
bugs?: {url: string}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
homepage
and bugs
were only declared on the top-level of Packument
before. Now they're declared on PackageJson
and Packument
uses Pick<PackumentVersion, "homepage" | "bugs">
.
@@ -23,6 +25,7 @@ export interface PackageJson { | |||
peerDependencies?: Dependencies; | |||
bundleDependencies?: Dependencies; | |||
bundledDependencies?: Dependencies; | |||
optionalDependencies?: ObjectOfStrings; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optionalDependencies
was only declared on ManifestVersion
before. Now it's declared on PackageJson
and ManifestVersion
uses Pick<PackumentVersion, "optionalDependencies">
.
maintainers: Maintainer[]; | ||
dist: Dist; | ||
readme?: string; | ||
readmeFilename?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readme
and readmeFilename
were only declared on the top-level of Packument
before. Now they're declared on PackumentVersion
and Packument
uses Pick<PackumentVersion, "homepage" | "bugs">
.
07daed0
to
be50483
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this makes most of my work in #26 redundant. If this gets merged we can probably just close my PR.
(Question: Would it make sense to add the type
field that I included in my PR?)
Instead of repeating fields that are hoisted to the top-level of the packument from the latest version published, or repeating fields that are common to both full and abbreviated metadata, can we use
Pick<T, K>
to list the hoisted/abbreviated fields?References
Depends on #15. I based this PR on #15 because this one depends on fixes from that one (e.g. the
license
andengines
fields are optional).