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

   ### SUMMARY
   Thanks to @mikebridge for the tip on this one.
   
   The `prettier-frontend` pre-commit hook's `entry` runs as `bash -c 
'<script>' file1 file2 ...` with no separator before the file args pre-commit 
appends. Under `bash -c` semantics, the first argument after the script string 
becomes `$0`, not part of `"$@"`, so the first staged frontend file is silently 
dropped from the `for file in "$@"` loop every time. With exactly one staged 
file, the loop body never runs at all, meaning prettier silently doesn't format 
it.
   
   Separately, the loop invoked `npx prettier --write` once per file instead of 
batching, which adds `npx`/config-resolution startup overhead per file rather 
than once per commit.
   
   This fix adds a `--` placeholder right after the script string to consume 
the `$0` slot (so every real file lands in `"$@"`), and replaces the per-file 
loop with one that just builds a path-stripped array, then calls prettier once 
with the full file list.
   
   ### TESTING INSTRUCTIONS
   Verified the bug and the fix empirically:
   
   ```
   $ bash -c 'echo "0=$0"; for file in "$@"; do echo "loop got: $file"; done' 
fileA.tsx fileB.tsx fileC.tsx
   0=fileA.tsx
   loop got: fileB.tsx
   loop got: fileC.tsx
   ```
   
   Confirms `fileA.tsx` never reaches the loop under the old entry.
   
   After the fix, with a placeholder consuming `$0`, all files land in the 
array and get passed to a single `prettier --write` call:
   
   ```
   $ bash -c 'cd superset-frontend && files=(); for f in "$@"; do 
files+=("${f#superset-frontend/}"); done; npx --yes [email protected] --write 
"${files[@]}"' -- superset-frontend/src/testdir/a.tsx 
superset-frontend/src/testdir/b.tsx
   src/testdir/a.tsx 31ms
   src/testdir/b.tsx 2ms
   ```
   
   Both test files (deliberately malformed) came back correctly formatted in 
one prettier invocation.
   
   ### ADDITIONAL INFORMATION
   - [ ] 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