Skip to content
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

fix(linter): create-package-json should omit non-npm and ignored packages #17883

Merged
merged 1 commit into from
Jun 30, 2023

Conversation

meeroslav
Copy link
Contributor

This PR fixes several issues that are blocking adoption of the dependency-checks rule:

  • Schema typo for ignoredDependencies
  • Ignore peer dependencies of ignored packages when collecting project dependencies
  • Ignore non-npm dependencies (this is a bug also for createPackageJson if project contains also non-JS code like nx package does)

Related Issue(s)

Fixes #

@meeroslav meeroslav added scope: core core nx functionality scope: linter Issues related to Eslint support in Nx labels Jun 30, 2023
@meeroslav meeroslav requested a review from a team as a code owner June 30, 2023 11:39
@meeroslav meeroslav self-assigned this Jun 30, 2023
@meeroslav meeroslav requested a review from a team as a code owner June 30, 2023 11:39
@vercel
Copy link

vercel bot commented Jun 30, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 30, 2023 11:41am

@@ -45,7 +45,7 @@ export default createESLintRule<Options, MessageIds>({
type: 'object',
properties: {
buildTargets: [{ type: 'string' }],
ignoreDependencies: [{ type: 'string' }],
ignoredDependencies: [{ type: 'string' }],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Optionally, instead of adding another param, we can expose peer dependencies as a nested structure under parent dependency, so that source function (e.g. linter rule) can easily discard all peer dependencies of the ignored packages.

@nx-cloud
Copy link

nx-cloud bot commented Jun 30, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 05740d1. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@@ -181,6 +181,7 @@ export function findProjectsNpmDependencies(
target: string,
options: {
helperDependencies?: string[];
ignoredDependencies?: string[];
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's better to delete this after the fact...

I also do not like the fact that helperDependencies is here.

const ignoredDependencies = ['react'];
const packageJson = createPackageJson(...);
for (const ignoredDep of ignoredDependencies) {
  delete packageJson.dependencies[ignoredDep];
  delete packageJson.devDependencies[ignoredDep];
  delete packageJson.peerDependencies[ignoredDep];
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Okay, it wouldn't work for transitive dependencies. I guess this is fine.

@github-actions
Copy link

github-actions bot commented Jul 6, 2023

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
scope: core core nx functionality scope: linter Issues related to Eslint support in Nx type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants