Re: [PR] fix(eslint-hook): ensure eslint hook receives arguments [superset]

[via GitHub]

alveifbklsiu259 commented on PR #32333:
URL: https://github.com/apache/superset/pull/32333#issuecomment-2675990981

   > Do we need the extra script file here, or does simply removing the sed 
business and letting the pre-commit hook run npm run lint (or even npm run 
lint-fix) from the pre-commit config do the job? I'm just hoping to reduce 
complexity/maintenance if it's not needed.
   
   I created a separate file because `$@` outputs ~~nothing~~(see below) when 
used in `pre-commit-config.yaml`, even if there are touched files under 
`superset-frontend` in the index. However, `$@` correctly outputs touched files 
when used in a separate script.
   
   > Oh weird, I'm pretty sure the $@ trick work on my machine, is this an 
OS-specific issue (like it doesn't work as expected on Windows/Linux)?
   
   I'm not sure if this is a OS-specific issue, I'm running on WSL2 with Ubuntu 
22.04.5.
   
   After a thorough test, I found that when `$@` is used in 
`pre-commit-config.yaml`, it does outputs touched files, but **not all of 
them**.
   
   `pre-commit-config.yaml`
   
   ```yaml
   - repo: local
       hooks:
         - id: eslint
           name: eslint
           entry: bash -c 'echo "$@"; exit 1'
           language: system
           pass_filenames: true
           files: \.(js|jsx|ts|tsx)$
   ```
   `superset-frontend/src/components/Avatar/index.tsx` is touched, but `$@` 
outputs nothing. (This is the reason why I initially thought `$@` outputs 
nothing)
   
   
![1](https://github.com/user-attachments/assets/db3e63ee-fb62-4279-a2eb-f390d4538138)
   
   When more files are added to the index, `$@` starts to output touched files, 
**but some files are missing**.
   
   `superset-frontend/src/components/Avatar/index.tsx` is missing from `$@`.
   
   
![2](https://github.com/user-attachments/assets/c4b80329-151d-4b2a-b504-ac1de2624b6c)
   
   `superset-frontend/src/components/Avatar/index.tsx` and 
`superset-frontend/src/components/Alert/index.tsx` are missing from `$@`.
   
   
![3](https://github.com/user-attachments/assets/3d6d1686-2e6d-45dd-b39c-caa80a029f9f)
   
   But if we use `$@` in a separate files, it seems to solve this issue.
   
   `scripts/eslint.sh`
   
   ```bash
   #!/usr/bin/env bash
   
   echo "$@"; exit 1
   ```
   
   `pre-commit-config.yaml`
   
   ```yaml
   - repo: local
       hooks:
         - id: eslint
           name: eslint
           entry: ./scripts/eslint.sh
           language: script
           pass_filenames: true
           files: \.(js|jsx|ts|tsx)$
   ```
   
   All touched files are included.
   
   
![4](https://github.com/user-attachments/assets/cdcffb14-df41-4b67-b25f-a8ddeb8b8640)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org