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

using the default .eslintrc.js does not lint any files that exist in .server or .client folders #9068

Closed
lifeiscontent opened this issue Mar 17, 2024 · 4 comments

Comments

@lifeiscontent
Copy link
Contributor

Reproduction

Using the vite template eslint fails to lint any files in app/.client or app/.server

System Info

System:
    OS: macOS 14.4
    CPU: (10) arm64 Apple M1 Max
    Memory: 3.00 GB / 64.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.11.1 - ~/.asdf/installs/nodejs/20.11.1/bin/node
    npm: 10.2.4 - ~/.asdf/plugins/nodejs/shims/npm
    pnpm: 8.15.4 - ~/.asdf/installs/nodejs/20.11.1/bin/pnpm
  Browsers:
    Chrome: 115.0.5790.114
    Safari: 17.4
  npmPackages:
    @remix-run/dev: 2.8.1 => 2.8.1 
    @remix-run/eslint-config: 2.8.1 => 2.8.1 
    @remix-run/node: 2.8.1 => 2.8.1 
    @remix-run/react: 2.8.1 => 2.8.1 
    @remix-run/serve: 2.8.1 => 2.8.1 
    vite: 5.1.6 => 5.1.6

Used Package Manager

npm

Expected Behavior

eslint lints correctly with default remix config

Actual Behavior

eslint lints incorrectly with default remix config

@brophdawg11
Copy link
Contributor

This is default eslint behavior when a folder begins with a dot. Their docs give you some options but the simplest IMO is to change your folder names so they don't start with a dot: app/whatever.client/

https://eslint.org/docs/latest/use/configure/ignore#the-eslintignore-file

Screenshot 2024-03-18 at 10 41 36 AM

@brophdawg11 brophdawg11 closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2024
@lifeiscontent
Copy link
Contributor Author

@brophdawg11 I'm specifically talking about updating the templates. you guys made changes to the tsconfig.json https://github.com/remix-run/remix/blob/main/templates/vite/tsconfig.json#L5-L8 why not the eslintrc?

@brophdawg11
Copy link
Contributor

Huh, I didn't know that had been added. IMO it feels a bit odd to configure templates not for stuff they already have, but for stuff that may be added in the future. It feels like a slippery slope that could bloat the template configs over time for tons of "maybe" use cases (see YAGNI).

That said, if it's in the TS configs, I don't mind adding it to the eslint setup for consistency. Want to open a PR?

It seems like it needs to be done via a negated --ignore-path since it can't be added in the list of files to lint at the end because it throws an error if files don't yet exist in those directories:

## Broken unless files exist in app/.server and app/.client
eslint --ignore-path .gitignore --ignore-pattern '!app/.server' --ignore-pattern '!app/.client' --cache --cache-location ./node_modules/.cache/eslint . app/.server app/.client

# Works even if files don't exist in app/.server and app/.client
eslint --ignore-path .gitignore --ignore-pattern '!app/.server' --ignore-pattern '!app/.client' --cache --cache-location ./node_modules/.cache/eslint .

@brophdawg11 brophdawg11 reopened this Mar 19, 2024
@lifeiscontent
Copy link
Contributor Author

@brophdawg11 yep, can do, I've figured out how to get this working in my own project, and ignorePatterns is a config key in the .eslintrc.cjs that can be used instead rather than passing CLI args

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants