-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add devDependencies support & regex ignored dependencies to @nx/dependency-checks
#30619
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
base: master
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
@nx/dependency-checks@nx/dependency-checks
|
@FrozenPandaz Any updates here? |
leosvelperez
left a comment
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.
Thanks for contributing!
Some general feedback:
- Look for operations that could be cached for all visitors. For example:
- The call to
findNpmDependenciescould be cached usingbuildTargetas the key, most of the time all configs will have the samebuildTarget - The few Regex you added could be created once instead of every time they are needed
- The call to
- Please do not remove existing comments unless you're addressing them or are outdated
| schema: { | ||
| type: 'array', | ||
| items: { |
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.
The current changes would allow multiple configurations with the same value for production and more than two configurations, which is not ideal. The tooling wouldn't prevent users from making such a mistake. We need to limit to at most two configurations, and a way to enforce that there are not two configurations with the same value for production.
Additionally, it would be great if users wouldn't have to repeat common configuration properties for both configurations.
|
Any progress on this? |
|
@PowerSupply Not for now, I will see whether I can pick this back up later this month. |
This (non-breaking) PR adds support for multiple configuration objects as well as a
productionflag to thedependency-checkseslint rule. This is similar to how https://knip.dev/features/production-mode works.With this change, it is now possible to enforce presence of
devDependenciesfor dev-only files, e.g..spec, like so:Furthermore, this PR also adds capability to match dependencies via regex pattern in the
ignoredDependencies.Current Behavior
It is impossible to enforce dev-only dependencies.
Expected Behavior
Related Issue(s)
Fixes #30614