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

Exclude Apps in affected scripts by tags from nx.json #1621

Closed
mehrad-rafigh opened this issue Jul 24, 2019 · 30 comments · Fixed by #10085
Closed

Exclude Apps in affected scripts by tags from nx.json #1621

mehrad-rafigh opened this issue Jul 24, 2019 · 30 comments · Fixed by #10085
Labels

Comments

@mehrad-rafigh
Copy link
Contributor

mehrad-rafigh commented Jul 24, 2019

Currently the only way to exclude apps and libs from being processed by the affected scripts, is by their names.
It would be useful, if we could exclude apps and libs in the affected scripts by their tags which can be defined in nx.json

For example having the following nx.json

....
    "booking": {
      "tags": ["booking"]
    },
    "error-page": {
      "tags": ["error-page"]
    },
    "my-nanosite": {
      "tags": ["my-nanosite", "nanosites"]
    },
    "my-other-nanosite": {
      "tags": ["my-other-nanosite", "nanosites"]
    },
...

and running npm run affected:apps -- --base=origin/develop --exclude="tags:nanosites" or something similar :) only apps should printed out, which are affected by changes and do not have the nanosites tag.
It should also be possible to pass a list of tags to be excluded.
This would save us a lot of headaches, because we have more than 50 nanosites and adding them by name in the exclude part is tedious and error prone.

I know the nx.json is used for linting, but could that purpose be extended?

@wesleygrimes
Copy link
Contributor

Hi @mehrad-rafigh, I am working on a new feature to have the ability to ignore glob patterns from the affected commands using either a .nxignore file or nx.json -> ignore property. Would this meet your needs?

PR #1585

@mehrad-rafigh
Copy link
Contributor Author

Hi @wesleygrimes, I looked over your pull request. It doesn't add the feature I am looking for.
I want to be able to exclude entire apps by their tag names from affected:apps,test,lint and the such for example.

@mehrad-rafigh
Copy link
Contributor Author

#1274 somehow relates to my feature request

@alfaproject
Copy link
Contributor

Yeah I was going to say this is a duplicate d:

@alfaproject
Copy link
Contributor

I'm currently applying this patch using patch-package to achieve what I need in my project:

diff --git a/node_modules/@nrwl/workspace/src/command-line/affected.js b/node_modules/@nrwl/workspace/src/command-line/affected.js
index ac6c7d7..87927e6 100644
--- a/node_modules/@nrwl/workspace/src/command-line/affected.js
+++ b/node_modules/@nrwl/workspace/src/command-line/affected.js
@@ -117,8 +117,8 @@ function affected(parsedArgs) {
 exports.affected = affected;
 function getProjects(target, parsedArgs, workspaceResults, all) {
     var projects = all
-        ? shared_1.getAllProjectsWithTarget(target)
-        : shared_1.getAffectedProjectsWithTarget(target)(shared_1.parseFiles(parsedArgs).files);
+        ? shared_1.getAllProjectsWithTarget(target, parsedArgs.tags)
+        : shared_1.getAffectedProjectsWithTarget(target, parsedArgs.tags)(shared_1.parseFiles(parsedArgs).files);
     return projects
         .filter(function (project) { return !parsedArgs.exclude.includes(project); })
         .filter(function (project) { return !parsedArgs.onlyFailed || !workspaceResults.getResult(project); });
@@ -277,6 +277,7 @@ var dummyOptions = {
     head: 'head',
     exclude: ['exclude'],
     files: [''],
+    tags: [],
     verbose: false
 };
 var nxSpecificFlags = Object.keys(dummyOptions);
diff --git a/node_modules/@nrwl/workspace/src/command-line/nx-commands.js b/node_modules/@nrwl/workspace/src/command-line/nx-commands.js
index 200d5c9..ebfa4c0 100644
--- a/node_modules/@nrwl/workspace/src/command-line/nx-commands.js
+++ b/node_modules/@nrwl/workspace/src/command-line/nx-commands.js
@@ -134,6 +134,12 @@ function withAffectedOptions(yargs) {
         type: 'array',
         coerce: parseCSV,
         default: []
+    })
+        .option('tags', {
+        describe: 'Only process projects with the given tag',
+        type: 'array',
+        coerce: parseCSV,
+        default: []
     })
         .options('only-failed', {
         describe: 'Isolate projects which previously failed',
diff --git a/node_modules/@nrwl/workspace/src/command-line/shared.js b/node_modules/@nrwl/workspace/src/command-line/shared.js
index f354aff..883fdbb 100644
--- a/node_modules/@nrwl/workspace/src/command-line/shared.js
+++ b/node_modules/@nrwl/workspace/src/command-line/shared.js
@@ -274,18 +274,21 @@ function readNxJson() {
     return config;
 }
 exports.readNxJson = readNxJson;
-exports.getAffected = function (affectedNamesFetcher) { return function (touchedFiles) {
+exports.getAffected = function (affectedNamesFetcher, tags = []) { return function (touchedFiles) {
     var workspaceJson = readWorkspaceJson();
     var nxJson = readNxJson();
     var projects = getProjectNodes(workspaceJson, nxJson);
     var implicitDeps = getImplicitDependencies(projects, workspaceJson, nxJson);
     var dependencies = deps_calculator_1.readDependencies(nxJson.npmScope, projects);
     var sortedProjects = topologicallySortProjects(projects, dependencies);
+    if (tags.length) {
+      sortedProjects = sortedProjects.filter(p => p.tags.some(t => tags.includes(t)));
+    }
     var tp = touched_1.touchedProjects(implicitDeps, projects, touchedFiles);
     return affectedNamesFetcher(sortedProjects, dependencies, tp);
 }; };
-function getAffectedProjectsWithTarget(target) {
-    return exports.getAffected(affected_apps_1.affectedProjectNamesWithTarget(target));
+function getAffectedProjectsWithTarget(target, tags = []) {
+    return exports.getAffected(affected_apps_1.affectedProjectNamesWithTarget(target), tags);
 }
 exports.getAffectedProjectsWithTarget = getAffectedProjectsWithTarget;
 exports.getAffectedApps = exports.getAffected(affected_apps_1.affectedAppNames);
@@ -303,12 +306,15 @@ function getAllProjectNamesWithTarget(target) {
     return getProjectNames(function (p) { return p.architect[target]; });
 }
 exports.getAllProjectNamesWithTarget = getAllProjectNamesWithTarget;
-function getAllProjectsWithTarget(target) {
+function getAllProjectsWithTarget(target, tags = []) {
     var workspaceJson = readWorkspaceJson();
     var nxJson = readNxJson();
     var projects = getProjectNodes(workspaceJson, nxJson);
     var dependencies = deps_calculator_1.readDependencies(nxJson.npmScope, projects);
     var sortedProjects = topologicallySortProjects(projects, dependencies);
+    if (tags.length) {
+      sortedProjects = sortedProjects.filter(p => p.tags.some(t => tags.includes(t)));
+    }
     return sortedProjects.filter(function (p) { return p.architect[target]; }).map(function (p) { return p.name; });
 }
 exports.getAllProjectsWithTarget = getAllProjectsWithTarget;

@mehrad-rafigh
Copy link
Contributor Author

mehrad-rafigh commented Jul 29, 2019

@alfaproject @wesleygrimes This looks like a start to a solution, which I would love to see in NX!

@wesleygrimes
Copy link
Contributor

@alfaproject @wesleygrimes This looks like a start to a solution, which I would love to see in NX!

I am adding this ability, although I think we will go a slightly different route. I will reference the PR when it is submitted.

@hoang-innomize
Copy link

@wesleygrimes how it going with the feature that you have been working on to allow custom NX to ignore files/directories when running affected commands? I am working on CI/CD stuff for our repo and wanted to ignore files from affected to redeploy apps, such as I don't want to redeploy if I just updated my tslint.json file on the project root folder

@wesleygrimes
Copy link
Contributor

@hoang-innomizetech Yes, you can now create an .nxignore file. This has been merged to master and should be available if not now, definitely in the next release.

@hoang-innomize
Copy link

I am using NX 8.4.13 and the .nxignore file is created but it seems not to work.

@wesleygrimes
Copy link
Contributor

Can you file an issue please and provide a small reproduction with example nxignore file?

@hoang-innomize
Copy link

@wesleygrimes I think we have an open issue #895, release 8.4.13 included that issue but it still open

@sroettering
Copy link

@alfaproject @wesleygrimes This looks like a start to a solution, which I would love to see in NX!

I am adding this ability, although I think we will go a slightly different route. I will reference the PR when it is submitted.

Any update on this?

@vsavkin vsavkin changed the title [Feature Request] Exclude Apps in affected scripts by tags from nx.json Exclude Apps in affected scripts by tags from nx.json Dec 5, 2019
@vsavkin vsavkin added the scope: core core nx functionality label Dec 5, 2019
@wickstargazer
Copy link

wickstargazer commented Feb 11, 2020

Hi,

I saw there is a pullrequest for .nxignore --> PR #1585

However it only works for the workspace-lint command and not the affected commands

Can it be added in? I see that its added in the core/file-utils.ts in the function allFilesInDir

However thats only use in the workspace-lint as i observed.

Can we add the ignore machanism for the affected as well?

From what I observe there are several places this mechanism can be coded in, if someone would guide with me it would be great.

  1. We either add a project level check with tags/flags at these functions
  • getTouchedProjects,
  • getImplicitlyTouchedProjects,
  • getImplicitlyTouchedProjectsByJsonChanges,
  • getTouchedProjectsInNxJson,
  • getTouchedProjectsInWorkspaceJson
  1. Or we can go directly for the files using glob pattern in .nxignore
  • Either in shared.parseFiles after the git command are run
  • Or add directly into the file-utils.calculateFileChanges

I like the

add directly into the file-utils.calculateFileChanges

option the most. Please advise :D, If so can i work on it and open a PR?

@johannesschobel
Copy link
Contributor

same here. I have the feeling, that the **/generated entry in my .nxignore file gets ignored (haha). In this case, the npm run affected:lint --fix --parallel --files=... command still tries to lint some auto-generated files (i.e., from a 3rd-party-codegen package). Obviously, this code is not lint-compliant and, therefore, cannot be committed properly :(

Does anyone have a solution for this?!

@github-actions github-actions bot added the stale label May 29, 2020
@nrwl nrwl deleted a comment from github-actions bot May 29, 2020
@FrozenPandaz
Copy link
Collaborator

Hi, sorry about this.

This was mislabeled as stale. We are testing ways to mark not reproducible issues as stale so that we can focus on actionable items but our initial experiment was too broad and unintentionally labeled this issue as stale.

@github-actions
Copy link

github-actions bot commented Feb 3, 2021

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Feb 3, 2021
@mehrad-rafigh
Copy link
Contributor Author

Unstale

@github-actions github-actions bot removed the stale label Feb 8, 2021
@github-actions
Copy link

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Mar 29, 2021
@mehrad-rafigh
Copy link
Contributor Author

Still..unstale

@alfaproject
Copy link
Contributor

alfaproject commented Mar 29, 2021

Another 3 months and I've been patching for 2 years. I know I should have made a proper PR back then but the project grew in complexity and the direction of this request wasn't clear and probably still isn't clear. I'm not sure why tho, seems like such a valid feature.

We have some beefy GitLab instances to test stuff like the Angular projects which have their own tag, and then smaller GitLab instances to test all the node services and libraries that we have. It's much easier to just do yarn affected:test --tags=angular (it's more complex than this because we split this even further into multiple jobs per type but you get the idea)

@github-actions github-actions bot removed the stale label Mar 29, 2021
@mehrad-rafigh
Copy link
Contributor Author

Unstale...still

@gagle
Copy link

gagle commented May 19, 2021

We can use the --exclude flag but this would be awesome to have...

@github-actions
Copy link

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Jan 25, 2022
@alfaproject
Copy link
Contributor

I still want this feature and I actually wonder if this would be good enough for our use case: #8364

I haven't read the code yet but from the title it sounds like it could be good enough for us. I'll have a better look this week at that feature

@github-actions github-actions bot removed the stale label Jan 26, 2022
@ChristianBarrientos
Copy link

Unstale...still

@github-actions
Copy link

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Dec 28, 2022
@johannesschobel
Copy link
Contributor

Keep this issue alive

@github-actions github-actions bot removed the stale label Dec 29, 2022
@charleskoehl
Copy link

I've created a workaround that partly addresses this issue. It allows me to conditionally execute nx-ignore depending on whether I include "nx-ignore" in the last commit message.

Let's say you have two apps you want to conditionally use nx-ignore with. Let's call them app1 and app2:

  1. Create a vercel-pre-deploy.sh file in the root of your monorepo.
#!/bin/sh

if git log -1 --pretty=%B | grep -q 'nx-ignore'; then
  npx nx-ignore "$1"
else
  echo "Skipping nx-ignore step because 'nx-ignore' was not found in the commit message."
  exit 1
fi
  1. In a terminal, execute chmod +x vercel-pre-deploy.sh in that directory.
  2. In the "Ignored Build Step" field of the Git settings for each app, enter the following, respectively:
    bash vercel-pre-deploy.sh app1
    bash vercel-pre-deploy.sh app2
  3. Now, whenever you want nx-ignore to be used by each app when you push a branch, just include nx-ignore somewhere in the last commit message.

@github-actions
Copy link

github-actions bot commented May 7, 2023

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.