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

Include parameter is ignored when no variable is used inside task #106

Open
fdipuma opened this issue Apr 22, 2024 · 5 comments
Open

Include parameter is ignored when no variable is used inside task #106

fdipuma opened this issue Apr 22, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@fdipuma
Copy link

fdipuma commented Apr 22, 2024

Version

0.6.4

Details

I was not super sure if to open this as a bug or as a feature request, still I think the expected behavior differs to how it works now.

If I use the include parameter inside a task, but no variable (e.g. {staged}) is used in the command args, the include filter is completely ignored and the task is executed always, even if there are no changes to files matched by the glob pattern.

e.g.

{
  "tasks": [    
    {
      "name": "npx-lint-staged",
      "group": "pre-commit",
      "command": "npx",
      "args": [
        "lint",
        "staged"
      ],
      "include": [
        "client/**/*"
      ]
    }
  ]
}

If I commit a change in another folder (e.g. server/test.txt), the task npx-lint-staged is executed.

Is this the intended behavior? I think it really confuses the user right now.

Thanks!

Steps to reproduce

  • create a task inside task-runner.json with an "include" parameter populated, but no variable in the command args
  • modify a file with a path not matched by the glob pattern used in step 1
  • commit changes

expected: the task created in step 1 is skipped because no file is matched
actual: the task created in step 1 is executed

@fdipuma fdipuma added the bug Something isn't working label Apr 22, 2024
@alirezanet
Copy link
Owner

Hi @fdipuma ,
unfortunately, I'm a little bit busy this week but I'll look into this the next week.

@fdipuma
Copy link
Author

fdipuma commented May 16, 2024

Hey @alirezanet , thanks, we are currently using a bad workround, something like this:

{
  "tasks": [    
    {
      "name": "npx-lint-staged",
      "group": "pre-commit",
      "command": "bash",
      "args": [
        "-c",
        "test -n \"${staged:-}\" && npx lint-staged" // workaround because include does not work without ${staged} var
      ],
      "include": [
        "client/**/*"
      ]
    }
  ]
}

But this generates a lot of issues when there are too many staged files:

--------------------------------------------------
[Husky] ⚡ Preparing task 'npx-lint-staged'
[Husky] ⚠️ The Maximum argument length '8191' reached, splitting matched files into 5 chunks
[Husky] ⌛ Executing task 'npx-lint-staged' ...
Command line is too long.
  ❌ Task 'npx-lint-staged' failed in 19ms
husky - pre-commit hook exited with code 1 (error)

Do you have any other workaround? Or some idea on when you could look at the issue?

I would also like to create a PR if you like, but I'm not 100% sure where to act.

@alirezanet
Copy link
Owner

alirezanet commented May 16, 2024

Hi @fdipuma,
sorry for the delay in getting back to you. You're right, this is a known issue. Currently, the Include/Exclude feature only functions with variables because it's challenging to determine whether the user intends to ignore a task or execute it. (since we're not limited to pre-commit hooks) I had overlooked this problem, but I'll make it a priority to find a solution soon. As you're directly dealing with this, do you have any suggestions? Please inform me if you think any particular/potential feature could address your needs.

@alirezanet
Copy link
Owner

alirezanet commented May 16, 2024

one potential fix for this would be, if we didn't have any variables in the arguments, we could consider using git staged files for the glob patterns (include/exclude) by default or with a configuration like "UseGitStagedFiles": true.
do you think this works for you?

@fdipuma
Copy link
Author

fdipuma commented May 16, 2024

@alirezanet yes, I agree with you that some property is less error prone from the user perspective. But I was thinking an enum could be better than a boolean prop:

{
  "tasks": [    
    {
      "name": "npx-lint-staged",
      "group": "pre-commit",
      "command": "npx",
      "fallbackFilteringRule": "git-staged",  // use this when no param is in the args, this could be an enum and the default value could be "none" if we want to keep compatibility, and other values could be "git-staged", "git-files", "all-files", and so on
      "args": [
        "lint",
        "staged"
      ],
      "include": [
        "client/**/*"
      ]
    }
  ]
}

WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants