FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix: ESLint 9+ path: handle child script errors in whitespace-async.js by dataCenter430 · Pull Request #3239 · airbnb/javascript · GitHub

/ javascript Public

Fix: ESLint 9+ path: handle child script errors in whitespace-async.js - #3239

Open
dataCenter430 wants to merge 4 commits into
airbnb:masterfrom
dataCenter430:fix/handle-child-script-errors-in-whitespace-async
ConversationCommits (4)ChecksFiles changed
Open

Fix: ESLint 9+ path: handle child script errors in whitespace-async.js#3239
dataCenter430 wants to merge 4 commits into
airbnb:masterfrom
dataCenter430:fix/handle-child-script-errors-in-whitespace-async

Conversation

dataCenter430 commented Mar 6, 2026

Copy link
Copy Markdown

Summary

When CLIEngine is not available (ESLint 9+), whitespace.js loads config by running whitespace-async.js with execSync and parsing its stdout as JSON. The async script had no .catch() on its promise, so rejections became unhandled: the child could exit non-zero and print to stderr while the parent received empty or partial stdout and failed with a generic JSON.parse error, hiding the real failure.

Approach

  • Do not add try/catch in whitespace.js; errors should still crash.
  • Fix the child script so the ESLint 9/10 path fails in a clear, diagnosable way.

Changes

  • packages/eslint-config-airbnb-base/whitespace-async.js: Add .catch() so that on rejection we log the error to stderr and process.exit(1).
  • packages/eslint-config-airbnb/whitespace-async.js: Same change.

On success, only JSON is written to stdout, so the parent’s execSync + JSON.parse continues to work. On failure, the real error goes to stderr and the process exits 1, so execSync throws and the thrown error includes the child’s stderr, making the root cause visible without masking crashes.

Fixes #3238

dataCenter430 commented Mar 6, 2026

Copy link
Copy Markdown
Author

Hey, @ljharb
I'm getting the CI testing fail, even though there is no conflict.
could you pls review the PR?
Thank you

ljharb requested changes Mar 7, 2026
View reviewed changes

ljharb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

All this does is exit when there’s an error - i thought the issue was an eslint 9+ bug that could be fixed?

dataCenter430 requested a review from ljharb March 7, 2026 13:42

dataCenter430 commented Mar 7, 2026

Copy link
Copy Markdown
Author

hey, @ljharb
thank you for your review.

All this does is exit when there’s an error - i thought the issue was an eslint 9+ bug that could be fixed?

I understood you. you really want fix the real ESLint 9+ bug so the config loads correctly; not just better behavior when it errors, right?
you’re right that the goal is to fix the underlying ESLint 9+ bug.
and pls don't close the PR. actually this is my first PR to contribute to Javascript and it's so important for me.
okay, I'll address this for sure

dataCenter430 commented Mar 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Hey, @ljharb
I fixed the underlying ESLint 9+ issue by making the async whitespace path run in legacy (eslintrc) mode and using the legacy ESLint class when available.
The parent now spawns whitespace-async.js with ESLINT_USE_FLAT_CONFIG=false, and the child uses loadESLint({ useFlatConfig: false }) (falling back to ESLint/calculateConfigForFile where needed) so our existing baseConfig is valid.
The result is that the ESLint 9+ path now successfully loads the whitespace config and still crashes on real errors instead of just exiting.

could you pls review the PR again? 🙏
Best regards.

dataCenter430 commented Mar 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Hi, @ljharb how are you?
There is no review yet for 2 weeks. 🥹 😭
I'm so afraid this will be ignored.
Could you please review my first PR when you have a chance?
Best regards

dataCenter430 commented Apr 1, 2026

Copy link
Copy Markdown
Author

Hi, @ljharb how are you doing?
I'm really sorry to ping you.
I thought you forgot my first contribution completely. 🫤
Would you please review my PR again?
I really appreciate your review with this.
Thank you.

daltino approved these changes Jun 27, 2026
View reviewed changes

daltino left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This PR updates the handling of ESLint versions to ensure compatibility with ESLint 9+ by dynamically checking and loading the appropriate class or functionality. The changes are minimal, clear, and address the issue effectively without introducing unnecessary complexity.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ljharb Awaiting requested review from ljharb
2 more reviewers
daltino daltino approved these changes
anandfresh anandfresh approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

No error handling around execSync + JSON.parse in whitespace.js (ESLint 9 path)

4 participants

Back | FazBrowse Home | New Git URL