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

Thu, 20 Feb 2025 09:37:11 -0800 


alveifbklsiu259 opened a new pull request, #32333:
URL: https://github.com/apache/superset/pull/32333

   fix(eslint-hook): ensure eslint hook receives arguments
   
   ### SUMMARY
   
   **Issue**
   For some reason, the `$@` variable is not correctly received by the eslint 
hook in `pre-commit-config.yaml`.
   
   **Solution**
   Moving the code to a separate file solves this issue.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   `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/Alert/index.tsx` is modified, but the hook 
receives nothing.
   
   ![eslint hook receives 
nothing](https://github.com/user-attachments/assets/499cd945-e3a2-4eda-82b0-16e3b6c5f7d3)
   
   This causes the eslint hook to always pass because no files are being passed 
to it.
   
   Now, set `entry` back to `bash -c 'cd superset-frontend && npm run eslint -- 
$(echo "$@" | sed "s|superset-frontend/||g")'`, and add this to the index:
   
   
![superset-frontend/src/components/Alert/index.tsx](https://github.com/user-attachments/assets/36b598fc-80bd-4764-823c-999cc7855046)
   
   Running eslint correctly catches the error.
   ![eslint 
error](https://github.com/user-attachments/assets/1181f1e5-9681-47b5-beae-597b35825ea8)
   
   But running eslint hook does not.
   
   ![eslint hook 
passed](https://github.com/user-attachments/assets/ebdba3c2-503f-4c5d-8ae7-f93d48ee0897)
   
   Moving the code to a separate file does solve the issue, it correctly 
receives the variable `$@`.
   `scripts/eslint.sh`
   
   ```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)$
   ```
   
   ![successfully receives 
args](https://github.com/user-attachments/assets/2dc908b9-450e-467e-a6fb-ce0f7ccf2e8c)
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to