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]