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
Allow publishing of packages with ignored devDependencies #907
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 736a89a 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 |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 736a89a:
|
didn't mean to close and reopen this I thought I was in a different PR and was getting confused why I couldn't merge the PR (and also why @KasFijolek approved) 😊 |
Might be worth looking at |
@@ -115,4 +115,39 @@ describe("getting the dependency graph", function() { | |||
`); | |||
}) | |||
); | |||
|
|||
it("should skip devDependencies if specified", function() { |
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 would also be great to have a test for the version command (I assume that it's where the original problem originated). While this package here might be directly responsible for the issue - it is an implementation detail and I would like to have a test for what people usually interact with (so @changesets/cli
)
packages/types/src/index.ts
Outdated
@@ -71,6 +71,8 @@ export type Config = { | |||
/** The minimum bump type to trigger automatic update of internal dependencies that are part of the same release */ | |||
updateInternalDependencies: "patch" | "minor"; | |||
ignore: ReadonlyArray<string>; | |||
/** Allows packages to be published if they have ignored packages in their devDependencies */ | |||
allowIgnoredDevDependencies?: boolean; |
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.
q: what would be the downsides of removing this flag entirely and instead just ignoring such dev dependencies at all times?
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.
This would be fine, and certainly would be fine if the dev dependency is marked as private.
This change would really improve my workflow 😄 |
@wycats I'm OK with this "feature" request - but I think that the additional config option that has been introduced here is redundant. Would you be able to prepare a PR targeting this PR that would introduce the requested changes? I'm a little bit busy right now and it might take me some days to get to this myself. |
@Andarist I reworked this removing the extra config option and made it so it just ignores devDependencies when looking at ignored files. I added a test for the I opted to not make it ignore devDependencies everywhere as devDependencies can have an effect on your package's output (they may affect how it is built) but it is fine for them to be ignored. |
@@ -73,5 +73,6 @@ | |||
"scripts/*" | |||
] | |||
}, | |||
"prettier": {} | |||
"prettier": {}, | |||
"packageManager": "yarn@1.22.19" |
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.
For those of us who use another version of yarn than classic and have corepack enabled this will use the right yarn version when working on this repo.
Resolves: #906