Skip to content

Conversation

@overlookmotel
Copy link
Member

@overlookmotel overlookmotel commented Jul 23, 2025

Add getFilename and getPhysicalFilename methods to Context, to match ESLint's API.

Copy link
Member Author

overlookmotel commented Jul 23, 2025


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@overlookmotel overlookmotel force-pushed the 07-23-feat_napi_oxlint_add_more_file_path-related_apis_to_context_ branch from d2a9782 to f503e4e Compare July 23, 2025 16:03
@overlookmotel overlookmotel requested a review from camc314 July 23, 2025 16:06
@overlookmotel overlookmotel marked this pull request as ready for review July 23, 2025 16:06

/**
* Get the absolute path of the file being linted.
* @deprecated Use `filename` instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think we should implement deprecated APIs, just for the sake of filling out the context object

Copy link
Member Author

@overlookmotel overlookmotel Jul 23, 2025

Choose a reason for hiding this comment

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

I figured these methods are part of ESLint's API, so we should include them, even though they're deprecated. An older ESLint plugin might otherwise work quite happily with Oxlint, but it'd break if it uses these APIs.

Copy link
Contributor

@camc314 camc314 Jul 24, 2025

Choose a reason for hiding this comment

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

Lets chat in a bit? I don't quite agree, especially since we don't have full compat with eslint, i don't think we should fill out deprecated APIs, we shuold instead push plugin maintainers to use the new APIs.

More of my reasoning is adding something deprecated, means that we definitly will remove it at some point which means a breaking change ect

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, we disagree here. I think we're aiming for ESLint compat, so we need to provide all ESLint's APIs, regardless of whether those APIs are deprecated or not.

But it's not important (yet). Feel free to close this if you disagree and we can always return to it later once we've confirmed that we are definitely going for ESLint compat.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah sorry to be a pain. I think let's close for now and return once it's confirmed.

@graphite-app graphite-app bot changed the base branch from 07-23-feat_napi_oxlint_add_id_to_context_ to graphite-base/12481 July 23, 2025 16:21
@graphite-app graphite-app bot force-pushed the graphite-base/12481 branch from 294ec37 to bc54d3c Compare July 23, 2025 16:29
@graphite-app graphite-app bot force-pushed the 07-23-feat_napi_oxlint_add_more_file_path-related_apis_to_context_ branch from f503e4e to 8c4e05d Compare July 23, 2025 16:29
@graphite-app graphite-app bot changed the base branch from graphite-base/12481 to main July 23, 2025 16:30
@graphite-app graphite-app bot force-pushed the 07-23-feat_napi_oxlint_add_more_file_path-related_apis_to_context_ branch from 8c4e05d to f49acdb Compare July 23, 2025 16:30
@overlookmotel overlookmotel changed the base branch from main to graphite-base/12481 July 24, 2025 10:59
@overlookmotel overlookmotel force-pushed the 07-23-feat_napi_oxlint_add_more_file_path-related_apis_to_context_ branch from f49acdb to e053438 Compare July 24, 2025 10:59
@overlookmotel overlookmotel changed the base branch from graphite-base/12481 to 07-24-feat_napi_oxlint_add_filename_property_to_context_ July 24, 2025 11:00
@overlookmotel overlookmotel changed the title feat(napi/oxlint): add more file path-related APIs to Context feat(napi/oxlint): add getFilename + getPhysicalFilename methods to Context Jul 24, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Jul 24, 2025

CodSpeed Instrumentation Performance Report

Merging #12481 will not alter performance

Comparing 07-23-feat_napi_oxlint_add_more_file_path-related_apis_to_context_ (e053438) with main (515b473)1

Summary

✅ 34 untouched benchmarks

Footnotes

  1. No successful run was found on 07-24-feat_napi_oxlint_add_filename_property_to_context_ (1e097b1) during the generation of this report, so main (515b473) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@graphite-app graphite-app bot changed the base branch from 07-24-feat_napi_oxlint_add_filename_property_to_context_ to graphite-base/12481 July 24, 2025 11:12
@graphite-app graphite-app bot force-pushed the 07-23-feat_napi_oxlint_add_more_file_path-related_apis_to_context_ branch from e053438 to edf72c3 Compare July 24, 2025 11:20
@graphite-app graphite-app bot force-pushed the graphite-base/12481 branch from 1e097b1 to 2401976 Compare July 24, 2025 11:20
@graphite-app graphite-app bot changed the base branch from graphite-base/12481 to main July 24, 2025 11:20
@graphite-app graphite-app bot force-pushed the 07-23-feat_napi_oxlint_add_more_file_path-related_apis_to_context_ branch from edf72c3 to 77731f0 Compare July 24, 2025 11:20
@camc314 camc314 closed this Jul 24, 2025
@overlookmotel overlookmotel deleted the 07-23-feat_napi_oxlint_add_more_file_path-related_apis_to_context_ branch July 25, 2025 12:49
@connorshea
Copy link
Contributor

We may want to start opening issues in ESLint plugins still using getFilename/getPhysicalFilename if we intend to keep this decision (or maybe even start opening PRs to fix plugins where possible):

❌ Output for rule "storybook/default-exports":

  x Error running JS plugin.
  | File path: /Users/connorshea/code/oxlint-jsplugin-checker/testfile.ts
  | TypeError: context.getFilename is not a function
  |     at Program:exit (file:///Users/connorshea/code/oxlint-jsplugin-checker/node_modules/.pnpm/[email protected][email protected][email protected][email protected]_@testing-libr_b0686cb35cfd040b26303d90bb3d9af4/node_modules/eslint-plugin-storybook/dist/index.js:748:67)
  |     at walkProgram (file:///Users/connorshea/code/oxlint-jsplugin-checker/node_modules/.pnpm/[email protected]/node_modules/oxlint/dist/plugins.js:9083:94)
  |     at lintFileImpl (file:///Users/connorshea/code/oxlint-jsplugin-checker/node_modules/.pnpm/[email protected]/node_modules/oxlint/dist/plugins.js:10031:63)
  |     at lintFile (file:///Users/connorshea/code/oxlint-jsplugin-checker/node_modules/.pnpm/[email protected]/node_modules/oxlint/dist/plugins.js:10001:10)
  |     at lintFileWrapper (file:///Users/connorshea/code/oxlint-jsplugin-checker/node_modules/.pnpm/[email protected]/node_modules/oxlint/dist/cli.js:411:9)

Link to storybook eslint plugin source: https://github.com/storybookjs/storybook/blob/v10.0.5/code/lib/eslint-plugin/src/rules/default-exports.ts#L102

@overlookmotel
Copy link
Member Author

overlookmotel commented Nov 6, 2025

Personally, I think we should reverse the decision and implement these APIs. IMO it's no big deal - they're all just another access method for data that existing getters on Context already provide.

Even if they disappear from ESLint in v10, there's no big problem I think with us supporting them indefinitely.

Trying to get every ESLint plugin to update is going to be an impossible task. Let's just accept the world as it is, warts and all!

Note: It's not just getFilename and getPhysicalFilename. There's a whole bunch of these deprecated get* methods.

@camc314
Copy link
Contributor

camc314 commented Nov 6, 2025

Lets chat monday?

@overlookmotel overlookmotel added the A-linter-plugins Area - Linter JS plugins label Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter-plugins Area - Linter JS plugins C-enhancement Category - New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants