workflow is attempting to lint deleted files

[imsteev]

hello! it's me again 😄

I'm getting a workflow error that says:

PR with files detected: someFile,anotherFile
##[error]Error: No files matching 'someFile' were found.
##[error]No files matching 'anotherFile' were found.

[bradennapier]

What settings are you using? This is the purpose of the errorOnUnmatchedPattern which should default to false.

[imsteev]

I agree that errorOnUnmatchedPattern defaulted to false should work.. the only setting we've toggled is reportWarnings 🤔

the following is our exact setup:

name: 'eslint'
on: [pull_request]

jobs:
  eslint:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - uses: bradennapier/eslint-plus-action@v3.4.0
        with:
          reportWarnings: false

[imsteev]

@bradennapier are you able to take a look? this issue is coming up quite often for us during things like refactoring. the only workaround to remove the failing check is to manually delete the Workflow results

I was originally going to suggest filtering the files fetched from the PR by checking file.status === 'deleted', but I don't think that is being return from the GraphQL request that is being made

[bradennapier]

@bradennapier are you able to take a look? this issue is coming up quite often for us during things like refactoring. the only workaround to remove the failing check is to manually delete the Workflow results

I was originally going to suggest filtering the files fetched from the PR by checking file.status === 'deleted', but I don't think that is being return from the GraphQL request that is being made

I was planning to look into this ASAP - that was my expected solution :-P

[bradennapier]

@imsteev see the PR #31 - mine works fine when I deleted src/quick.ts

What does the eslintconfig for your project look like? Are you potentially enabling the errorOnUnmatched there?

Also - what version of eslint are you using? Looks like the errorOnUmatchedPattern was added in 6.8.0

[imsteev]

@bradennapier our config file looks like:

module.exports = {
  extends: [
    'react-app',
    'prettier',
    'plugin:@typescript-eslint/recommended'
  ],
  plugins: ['react-hooks', 'mocha', 'i18next', '@typescript-eslint'],
  ignorePatterns: ['build/**'],
  rules: {
    ...
  },
  overrides: [
    {
      files: ['*.test.ts', '*.test.tsx', '*.spec.ts'],
      rules: {
        ...
      }
    }
  ]
};

and the debug output before running ESLint:

[ESLINT] Run With Configuration  {
  extensions: [ '.js', '.jsx', '.ts', '.tsx' ],
  ignore: true,
  useEslintrc: true,
  rulePaths: [],
  errorOnUnmatchedPattern: false,
  fix: false,
  configFile: undefined
}

[imsteev]

Also - what version of eslint are you using? Looks like the errorOnUmatchedPattern was added in 6.8.0

oh wait this might be it... we're on 6.7.2

[imsteev]

@bradennapier alright bumping the version of ESLint to latest works for me (meaning the workflow passed with a deleted file!)

the downside to this is that I have to add SKIP_PREFLIGHT_CHECK=true to ignore CRA's dependency check before a yarn start.

[bradennapier]

Ah yeah - quick google shows thats a pretty big overall annoyance people have ... not much can be done there there :-\ I could do a force yarn add on eslint after you checkout but that prob would have unwanted side effects :-)

[bradennapier]

You could add a command in the workflow right before the linting runs that updates eslint though, assuming that wouldn't cause any unwanted changes. It doesn't look like anything breaking between the two versions.

I edited that in the issue so the indenting is probably wrong FYI

name: "lint"
on: [pull_request]
jobs:
  lint:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2
    - name: Update eslint 
      run: yarn add --dev eslint@6.8.0
    - uses: bradennapier/eslint-plus-action@v3.4.0
      with: 
         reportWarnings: false

[imsteev]

yeah any time i have to change a react-scripts dependencies, that SKIP_PREFLIGHT_CHECK flag is necessary. the workflow edit you mentioned seems promising, i didn't know you could add intermediate steps

[bradennapier]

Yep, you would actually want to do more than that, we skip installing your dependencies if we find node_modules exists when the action is called so you can also use that to cache your modules. If that would cause errors with CRA you can add the SKIP_PREFLIGHT_CHECK flag to your env for any of the steps with an env property on any given step as well.

https://github.com/bradennapier/eslint-plus-action/blob/1601d2e4f90de971fb03b2134f9c8d337f7ee195/.github/workflows/test.yml

name: "lint"
on: 
  # by adding a schedule task to this workflow we will automatically
  # begin serializing read-only runs and handling them. The cron job
  # below is set to run every 15 minutes, GitHub will ignore anything
  # under 10 minutes and run every 10 minutes anyway.
  schedule:
    - cron: '*/15 * * * *'
  pull_request:
    types:
      - opened
      - synchronize
      - closed
jobs:
  lint:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2
    - name: Get yarn cache directory path
      id: yarn-cache-dir-path
      run: echo "::set-output name=dir::$(yarn cache dir)"

    - uses: actions/cache@v2
      id: yarn-cache # use this to check for `cache-hit` (`steps.yarn-cache.outputs.cache-hit != 'true'`)
      with:
        path: ${{ steps.yarn-cache-dir-path.outputs.dir }}
        key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
        restore-keys: |
          ${{ runner.os }}-yarn-

    - name: Install dependencies
      run: yarn --prefer-offline && yarn add --dev eslint@6.8.0
      env:
        NODE_ENV: development
    - uses: bradennapier/eslint-plus-action@v3.4.0
      with: 
        reportWarnings: false

Haven't tested that lol but it should work - i dont know if caching would get confused by the fact the lock file would be adjusted there... but i think it should be fine because it would actually cache after the lock is updated --

[imsteev]

@bradennapier thanks for this info. I think I have a solution that works, but it jumps through a lot of hoops just to get this Workflow to run.

I wonder if a long-term solution that supports backwards compatibility is to just filter out all the files in the PR/diff that are deleted, and not pass them to ESLint at all. Otherwise I think it'd be helpful to document that this action requires 6.8.0 and above for errorOnUnmatchedPattern

[bradennapier]

Yeah, the problem is that isn't very easy to do since we don't get that meta from graphql -- there was an important reason for the original dev using the graphql instead of the rest request but I can't remember what it was. Kind of sucks you can't get that information from it though.

Happy to consider other options -- iterating all the changed files and checking if they exist doesn't seem ideal.