Skip to content

fix: avoid requiring eslint package metadata#4010

Closed
camc314 wants to merge 1 commit into
jsx-eslint:masterfrom
camc314:c/oxlint
Closed

fix: avoid requiring eslint package metadata#4010
camc314 wants to merge 1 commit into
jsx-eslint:masterfrom
camc314:c/oxlint

Conversation

@camc314

@camc314 camc314 commented May 15, 2026

Copy link
Copy Markdown

fixes oxc-project/oxc#17734 (comment)

There are scenarios when eslint will not be available - e.g. when using oxlint.

This PR adds some safety when requiring eslint to avoid message.js throwing an error, and preventing oxlint user from using this plugin.

cc @xsjcTony

Copilot AI review requested due to automatic review settings May 15, 2026 08:39
@codecov

codecov Bot commented May 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.69%. Comparing base (c9a2de7) to head (1861533).

Files with missing lines Patch % Lines
lib/util/message.js 66.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4010      +/-   ##
==========================================
- Coverage   97.71%   97.69%   -0.02%     
==========================================
  Files         137      137              
  Lines       10188    10192       +4     
  Branches     3797     3798       +1     
==========================================
+ Hits         9955     9957       +2     
- Misses        233      235       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a defensive fallback for environments where eslint isn’t installed (e.g., oxlint), preventing lib/util/message.js from throwing when trying to read ESLint’s package metadata.

Changes:

  • Wrap require('eslint/package.json') in a try/catch to tolerate missing ESLint installations
  • Switch message selection logic to depend on an optional eslintVersion value

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/util/message.js
try {
eslintVersion = require('eslint/package.json').version; // eslint-disable-line global-require
} catch (error) {
if (!error || error.code !== 'MODULE_NOT_FOUND') {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this seems unnecessary - the only thing that can throw here is requireing eslint/package.json

Comment thread lib/util/message.js

@ljharb ljharb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is an eslint plugin, and if something wants to support eslint plugins, it shouldn't require any changes in the eslint plugin, no matter what it's doing.

If we can't detect the eslint version, the plugin can't function properly, and its use is not supported. Perhaps oxlint can rewrite specifiers to produce the appropriate eslint package.json?

@ljharb ljharb closed this May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

linter/plugins: Possible dependency on the eslint package in some cases

3 participants