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

affected:lint with lint-staged hangs in nx monorepo #869

Closed
xmlking opened this issue Nov 1, 2018 · 25 comments
Closed

affected:lint with lint-staged hangs in nx monorepo #869

xmlking opened this issue Nov 1, 2018 · 25 comments
Assignees

Comments

@xmlking
Copy link
Contributor

xmlking commented Nov 1, 2018

I am using affected:lint with lint-staged and noticed it hangs forever when I run npx lint-staged.
wonder if I am doing something wrong.

The command npm run affected:lint -- --uncommitted --fix --parallel itself runs fine.

  "lint-staged": {
    "{apps,libs}/**/*.{ts,json,md,scss}": [
      "npm run affected:lint  -- --uncommitted --fix --parallel",
      "npm run format:write -- --uncommitted",
      "git add"
    ]
  },

Ref:
lint-staged/lint-staged#522

@DUBANGARCIA
Copy link

Hey guys in my case I have the following settings.

.huskyrc.json

{
	"hooks": {
		"pre-commit": "yarn lint-staged -c lint-staged.config.js --debug",
		"commit-msg": "yarn commitlint -E HUSKY_GIT_PARAMS",
		"pre-push": "yarn run lint && yarn run test && yarn run e2e"
	}
}

lint-staged.config.js

module.exports = {
	'*.ts': ['sh ./scripts/lint.sh 1', 'git add'],
	'*.{js,json,scss,css,md,html}': ['sh ./scripts/lint.sh 0', 'git add']
};
#!/usr/bin/env bash

SHOULD_LINT=$1
shift
SOURCES=$@
DESTINATIONS=""
DELIMITER=""
FIRST=1

set -e

for src in $SOURCES; do
  if [ "${FIRST}" -eq "1" ]; then
    DELIMITER=""
    FIRST=0
  else
    DELIMITER=","
  fi
  DESTINATIONS="$DESTINATIONS$DELIMITER${src}"
done

echo "$DESTINATIONS"
yarn run format:write --files="$DESTINATIONS"

if [ "${SHOULD_LINT}" -eq "1" ]; then
  yarn run affected:lint --fix --maxParallel=1 --parallel=false --files="$DESTINATIONS";
fi

exit;

Create this script to be able to format the files that really had been changed according to git, since with the flag of "--uncommitted" all will be formatted with prettier, but that could interfere with the rules of tslint, that is why it required that they will pass the files that have changed according to the lint-staged filters. However, it does not work either since the process never ends, although I think that it would be the fault of the commando's parallelism, but it still does not work, I hope this will also help to solve the solution.

@JoA-MoS
Copy link
Contributor

JoA-MoS commented Nov 20, 2019

here is how I think i solved this if anyone is interested

lint-staged.config.js

module.exports = {
  '*': (files) => [`nx format:write --files=${files.join(',')}`, `git add ${files.join(' ')}`],
};

@DUBANGARCIA
Copy link

@JoA-MoS, I had tried that too, but it didn't work, I will try again to try

@vsavkin vsavkin added the scope: core core nx functionality label Dec 4, 2019
@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.

@vsavkin vsavkin added scope: linter Issues related to Eslint support in Nx and removed scope: core core nx functionality labels Sep 3, 2020
@vsavkin
Copy link
Member

vsavkin commented Sep 3, 2020

Folks, could you provide a repo with a repro using the latest version of Nx? Thank you!

@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! 🙏

@JoA-MoS
Copy link
Contributor

JoA-MoS commented Sep 12, 2021

I ended up back here and thought I would post an update. Adding a lint-staged config with these settings seems to work

{
  "*{.js,.ts}": "nx affected:lint --fix --files=",
  "*": "nx format:write --files="
}

@VictorienTardif
Copy link

I ended up back here and thought I would post an update. Adding a lint-staged config with these settings seems to work

{
  "*{.js,.ts}": "nx affected:lint --fix --files=",
  "*": "nx format:write --files="
}

Hi @JoA-MoS I'd love to know how you got this working because with this exact config I always encounter an error:

✔ Preparing...
⚠ Running tasks...
  ❯ Running tasks for *.{js,ts}
    ✖ nx affected:lint --fix --files= [FAILED]

...
TypeError: path must not be empty

@JoA-MoS
Copy link
Contributor

JoA-MoS commented Sep 21, 2021

Sorry, I forgot that this was posted. I ended up just using eslint that command for lint didn't work the prettier command does.

So just running npx eslint --cache --fix was the solution I used.

https://github.com/semantic-release-plus/semantic-release-plus/blob/alpha/.lintstagedrc.json

It might actually be a bug that lint affected doesn't work the same format

@VictorienTardif
Copy link

Yep. Indeed nx format:write --files= works but nx affected:lint --fix --files= does not so it could be considered as a bug.
Anyway, I ended up using eslint directly also.
Thx for the quick reply!

@ggwzrd
Copy link

ggwzrd commented Oct 20, 2021

The same goes for nx affected:test --bail --files= it also returns the exact same error TypeError: path must not be empty

@miestasniyo
Copy link

What is the solution? 👀

@fs-innonova
Copy link

The only way I got it working was using this (default branch name is main):

"lint-staged": {
   "{apps,libs}/**/*.{ts,tsx,json,md,scss,html}": [
     "nx format:write --base main",
     "npm run affected:lint -- --base main"
   ]
 }

but why do I need to specify the branch name here? if in nx.json I already have this:

"affected": {
  "defaultBase": "main"
},

Am I missing some configuration in nx.json or the only way is to hardcode the branch name inside the lint-staged script?

@meeroslav
Copy link
Contributor

@fs-innonova can you provide minimal reproduction?

The passing of the base should not be necessary and you are correct it should be read from the default configuration. Running the affected command without specifying the base is how it's intended on the local machine.

On the other point, unless you provide --files param and let lint-staged inject the filenames there, you are using actually just a plain nx affected without lint-staged.

By running

yarn nx affected:lint --files packages/workspace/src/command-line/shared.ts packages/workspace/src/command-line/run-one.ts

inside the nx repo having these two files changed I can see the affected command properly picking them up and finding all their dependents.

Also, have in mind that lint-staged is intended to only lint staged files, ignoring possible lint issues in the rest of the application. Nx will always run on the entire project, even if just one file was changed.

@JoA-MoS
Copy link
Contributor

JoA-MoS commented Jan 25, 2022

EDIT: Updated solution here #869 (comment)

If anyone ends up here again lint-staged is using the full file name not the relative file name. Adding --relative https://github.com/okonet/lint-staged#command-line-flags when invoking lint-staged and using js config to concatenate the file names solve the issue for me

.husky/pre-commit

#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

npx lint-staged --relative 

lint-staged.config.js

module.exports = {
  '*': (filesArray) => {
    const files = filesArray.join();
    return [
      `nx affected:lint --fix --files=${files}`,
      `nx format:write --files=${files}`,
    ];
  },
};

I am a little concerned about the length of the command when making large changes, but this has worked for me a few times

If you want to try to figure out what is going on lint-staged has a --debug flag, from what I see when using the standard json config it is not sending the files for affected:lint but it is for format:write

@JoA-MoS
Copy link
Contributor

JoA-MoS commented Jan 25, 2022

@meeroslav I found the cause of the issue of why lint-staged works with format:write but not affected:lint lint-staged inserts a space between the end of the command and the file list https://github.com/okonet/lint-staged/blob/026aae0d3e3e7644ed55cdf2dd6c17c9d4c50da8/lib/resolveTaskFn.js#L105

It seems format:write is able to handle this but not affected:lint

This can be reproduced by running nx commands in the same way

This command works

nx format:write --files= .lintstagedrc.json temp-lint-staged.config.js .husky/pre-commit

This command does not

nx affected:lint --fix --files= .lintstagedrc.json temp-lint-staged.config.js .husky/pre-commit 

Please note the space after the equals sign

Should I create a new issue for NX for this?

@JoA-MoS
Copy link
Contributor

JoA-MoS commented Jan 25, 2022

As an FYI, I chatted with lint-staged author and line length could become an issue especially on windows lint-staged/lint-staged#1097 but using a function as shown in my previous comment is a good approach #869 (comment)

@meeroslav
Copy link
Contributor

meeroslav commented Jan 26, 2022

@JoA-MoS, the signature --files= something is incorrect as variable value needs to follow equals sign if one is added. So this is not an issue in Nx. I'm wondering how format:write works though.

These are correct:

# with equals
--files=first,second,third

# without equals
--files first second third

The approach with concatenation or using function as mentioned by @iiroj should be a way to go.

@JoA-MoS
Copy link
Contributor

JoA-MoS commented Jan 26, 2022

Thanks @meeroslav for confirming the = is not necessary. To amend the previous solution there is no need to use the lint-staged js configuration the following should work, and long list of files that exceed the command length max will be chunked correctly

.husky/pre-commit

#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

npx lint-staged --relative 

.lintstagedrc.json

{
  "*": ["nx affected:lint --fix --files", "nx format:write --files"]
}

@sourcier
Copy link

The above is not working for me I get the following error:

❯ yarn run lint-staged
yarn run v1.22.17
$ /Users/rrajaratnam/Tresors/Code/2022/sourcier-web/node_modules/.bin/lint-staged
✔ Preparing lint-staged...
⚠ Running tasks for staged files...
  ❯ .lintstagedrc.json — 2 files
    ❯ * — 2 files
      ✖ nx affected:lint --fix --files [FAILED]
      ◼ nx format:write --files
↓ Skipped because of errors from tasks. [SKIPPED]
✔ Reverting to original state because of errors...
✔ Cleaning up temporary files...

✖ nx affected:lint --fix --files:
nx affected:lint

Lint projects affected by changes

Run command using --base=[SHA1] (affected by the committed, uncommitted and untracked changes):
  --base  Base of the current branch (usually main)  [string]

or using --base=[SHA1] --head=[SHA2] (affected by the committed changes):
  --base  Base of the current branch (usually main)  [string]
  --head  Latest commit of the current branch (usually HEAD)  [string]

or using:
  --files        Change the way Nx is calculating the affected command by providing directly changed files, list of files delimited by commas  [array]
  --uncommitted  Uncommitted changes  [boolean]
  --untracked    Untracked changes  [boolean]

Options:
  --help           Show help  [boolean]
  --version        Show version number  [boolean]
  --parallel       Max number of parallel processes [default is 3]  [string]
  --all            All projects  [boolean]
  --exclude        Exclude certain projects from being processed  [array] [default: []]
  --runner         This is the name of the tasks runner configured in nx.json  [string]
  --skip-nx-cache  Rerun the tasks even when the results are available in the cache  [boolean] [default: false]
  --configuration  This is the configuration to use when performing tasks on projects  [string]
  --only-failed    Isolate projects which previously failed  [deprecated: The command to rerun failed projects will appear if projects fail. This now does nothing and will be removed in v15.] [boolean] [default: false]
  --verbose        Print additional error stack trace on failure

Examples:
  affected:lint --parallel=5                     Run lint in parallel
  affected:lint --all                            Run the lint target for all projects
  affected:lint --files=libs/mylib/src/index.ts  Run lint for all the projects affected by changing the index.ts file
  affected:lint --base=main --head=HEAD          Run lint for all the projects affected by the changes between main and HEAD (e.g., PR)
  affected:lint --base=main~1 --head=main        Run lint for all the projects affected by the last commit on main

Find more information and examples at https://nx.dev/cli/affected-lint

RangeError: path should be a `path.relative()`d string, but got "/Users/rrajaratnam/Tresors/Code/2022/sourcier-web/.lintstagedrc.json"
    at throwError (/Users/rrajaratnam/Tresors/Code/2022/sourcier-web/node_modules/ignore/index.js:364:9)
    at checkPath (/Users/rrajaratnam/Tresors/Code/2022/sourcier-web/node_modules/ignore/index.js:383:12)
    at Ignore._test (/Users/rrajaratnam/Tresors/Code/2022/sourcier-web/node_modules/ignore/index.js:504:5)
    at Ignore.ignores (/Users/rrajaratnam/Tresors/Code/2022/sourcier-web/node_modules/ignore/index.js:549:17)
    at /Users/rrajaratnam/Tresors/Code/2022/sourcier-web/node_modules/@nrwl/workspace/src/core/file-utils.js:34:41
    at Array.filter (<anonymous>)
    at calculateFileChanges (/Users/rrajaratnam/Tresors/Code/2022/sourcier-web/node_modules/@nrwl/workspace/src/core/file-utils.js:34:19)
    at projectsToRun (/Users/rrajaratnam/Tresors/Code/2022/sourcier-web/node_modules/@nrwl/workspace/src/command-line/affected.js:91:108)
    at Object.<anonymous> (/Users/rrajaratnam/Tresors/Code/2022/sourcier-web/node_modules/@nrwl/workspace/src/command-line/affected.js:25:26)
    at Generator.next (<anonymous>)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
~/T/Code/2022/sourcier-web develop +3 ❯                                                         ▼ 02:30:38

@piet-v
Copy link

piet-v commented May 5, 2022

@sourcier this happens if you omit the --relative flag?

@DaSchTour
Copy link
Contributor

Thanks @meeroslav for confirming the = is not necessary. To amend the previous solution there is no need to use the lint-staged js configuration the following should work, and long list of files that exceed the command length max will be chunked correctly

.husky/pre-commit

#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

npx lint-staged --relative 

.lintstagedrc.json

{
  "*": ["nx affected:lint --fix --files", "nx format:write --files"]
}

I added the exact configuration and my commits are not going through. I had the impression that it was working for some time, but suddenly it doesn't work anymore. Although a nx affected:lint runs through quite fast.

@meeroslav
Copy link
Contributor

@DaSchTour, I don't have much experience with lint-staged and --relative flag, but nx already provides the same functionality that lint-staged does with --uncommitted.

@Sloebel
Copy link

Sloebel commented Jan 18, 2023

affected:lint is project base, meaning that a project that is affected by a staged file will run lint.
I wrote the following solution that only lint the staged files

.lintstagedrc.js

const workspace = require('./workspace.json');

module.exports = {
	'*': ['nx format:write --files'],
	'**/src/**/*.{js,jsx,ts,tsx}': allStagedFiles => {
		const filesProjectMap = new Map(
			Object.keys(workspace.projects)
				.map(projectName => [
					projectName,
					allStagedFiles.filter(fileName => fileName.includes(`/${projectName}/`))
				])
		);

		return [...filesProjectMap]
			.filter(([project, files]) => files.length)
			.map(([projectName, files]) => `nx lint ${projectName} --lintFilePatterns=${files.join(',')}`);
	}
};

@github-actions
Copy link

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 Mar 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests