chore: replace ESLint with oxlint - #2641
Open
joyenjoyer wants to merge 8 commits into
Open
Conversation
joyenjoyer
marked this pull request as ready for review
July 15, 2026 13:44
joyenjoyer
force-pushed
the
instui-5035-oxlint-migration
branch
from
July 15, 2026 14:18
435bcbb to
65cacf4
Compare
Contributor
|
Contributor
Visual regression report
Diff images (33)alert.png — baseline no longer producedavatar.png — baseline no longer producedbadge.png — baseline no longer producedbillboard.png — baseline no longer producedbreadcrumb.png — baseline no longer producedbutton-and-derivatives.png — baseline no longer producedbyline.png — baseline no longer producedcalendar.png — baseline no longer producedcheckbox.png — baseline no longer producedcheckboxgroup.png — baseline no longer producedcolorpicker.png — baseline no longer producedcontextview.png — baseline no longer producedcustom-and-lucide-icons.png — baseline no longer produceddateinput-dateinput2.png — baseline no longer produceddatetimeinput.png — baseline no longer produceddiff-demo.png — 6324 pixels differdrilldown.png — baseline no longer producedfiledrop.png — baseline no longer producedform-errors.png — baseline no longer producedheading.png — baseline no longer producedimg.png — baseline no longer producedlink.png — baseline no longer producedmenu.png — baseline no longer producedmetric-pill-tag-timeselect-text.png — baseline no longer producedoptions.png — baseline no longer producedpagination.png — baseline no longer producedprogressbar.png — baseline no longer producedselect-simpleselect.png — baseline no longer producedtable.png — baseline no longer producedtabs.png — baseline no longer producedtooltip.png — baseline no longer producedtreebrowser.png — baseline no longer producedview.png — baseline no longer producedBaselines come from the |
joyenjoyer
force-pushed
the
instui-5035-oxlint-migration
branch
2 times, most recently
from
July 16, 2026 08:58
b1a8614 to
1429aaf
Compare
balzss
reviewed
Jul 20, 2026
balzss
left a comment
balzss
left a comment
Contributor
There was a problem hiding this comment.
the changeset look ok and linting works fine for me, however this breaks my editor integration which showed realtime feedback about lint errors. i can work around it and find a plugin that fills this gap but it worths checking if other editors like webstorm and vscode support it or not
matyasf
self-requested a review
July 20, 2026 15:28
matyasf
requested changes
Jul 20, 2026
| */ | ||
| export function hasNoticeHeader(sourceText) { | ||
| const leading = String(sourceText).replace(/\r\n/g, '\n').slice(0, CHARS) | ||
| return new RegExp(MUST_MATCH).test(leading) |
Collaborator
There was a problem hiding this comment.
Why just match this short text? All text must be included
Contributor
Author
There was a problem hiding this comment.
Eslint-plugin-notice also scans the first 1000 characters by default that is why I implemented this way.
Collaborator
There was a problem hiding this comment.
But here here are comparing this the MUST_MATCH text, not 1000 characters
Contributor
Author
There was a problem hiding this comment.
Fixed, full codebase linting became slower though.
| "files": ["**/*"], | ||
| "excludeFiles": ["**/__tests__/**"], | ||
| "plugins": ["jsx-a11y"], | ||
| "rules": { |
Collaborator
There was a problem hiding this comment.
Do we really need such huge config for each plugin? Why not use some defaults?
Contributor
Author
There was a problem hiding this comment.
I am going to revisit all the rules in a separate PR as we discussed. Please accept this PR.
Comment on lines
+370
to
+375
| "files": ["packages/ui-truncate-text/src/TruncateText/__tests__/**"], | ||
| "plugins": ["vitest"], | ||
| "rules": { | ||
| "vitest/valid-expect": "off" | ||
| } | ||
| }, |
Collaborator
There was a problem hiding this comment.
Why is there a rule for just this one?
Contributor
Author
There was a problem hiding this comment.
This was added to bypass this oxlint rule flagging an old Chai style assertion, I updated these assertions in TruncateText test files and removed this rule bypass from the config file.
| "ts-ignore": "allow-with-description" | ||
| } | ||
| ], | ||
| "@typescript-eslint/no-array-constructor": "error", |
Collaborator
There was a problem hiding this comment.
Cant we just use defaults for these? For most cases it would be much cleaner IMO
Contributor
Author
There was a problem hiding this comment.
I am going to revisit all the rules in a separate PR as we discussed. Please accept this PR.
There was a problem hiding this comment.
Pull request overview
This PR migrates the repo’s JavaScript/TypeScript linting from ESLint to oxlint, including porting previously custom ESLint rules to oxlint’s JS-plugin system and rewiring repo scripts to run oxlint instead of ESLint.
Changes:
- Adds root and
regression-testoxlint configs, including JS-plugins to replace the priornotice/noticeand@instructure/no-relative-importsESLint rules. - Updates scripts (
lint-staged,ui-scripts lint,regression-test lint) to runoxlint, and removes the legacy ESLint config/custom plugin. - Cleans up stale
eslint-disabledirectives and relocates theui-scriptslint command module.
Reviewed changes
Copilot reviewed 23 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/oxlint/notice-plugin.mjs | Adds an oxlint JS-plugin implementing the repo’s MIT notice header rule with an auto-fix. |
| scripts/oxlint/no-relative-imports-plugin.mjs | Adds an oxlint JS-plugin to block cross-package relative imports (replacement for the removed ESLint plugin). |
| scripts/oxlint/node_tests/notice-plugin.test.ts | Adds unit tests for notice plugin pure logic and rule visitor behavior. |
| scripts/oxlint/node_tests/no-relative-imports-plugin.test.ts | Adds unit tests for no-relative-imports plugin pure logic and rule visitor behavior. |
| scripts/oxlint/node_tests/testfixtures/no-relative-imports/pkg-a/package.json | Fixture package manifest for cross-package import tests. |
| scripts/oxlint/node_tests/testfixtures/no-relative-imports/pkg-a/src/index.ts | Fixture file containing imports that should/shouldn’t be flagged. |
| scripts/oxlint/node_tests/testfixtures/no-relative-imports/pkg-a/src/sibling.ts | Fixture sibling module import (allowed case). |
| scripts/oxlint/node_tests/testfixtures/no-relative-imports/pkg-b/package.json | Fixture package manifest for the “other package” side of the test. |
| scripts/oxlint/node_tests/testfixtures/no-relative-imports/pkg-b/src/index.ts | Fixture module imported via cross-package relative path (disallowed case). |
| scripts/eslint/no-relative-imports-plugin.mjs | Removes the legacy ESLint custom plugin implementation. |
| regression-test/package.json | Switches the regression-test lint script from ESLint to oxlint. |
| regression-test/.oxlintrc.json | Adds a regression-test-specific oxlint config extending the root configuration. |
| packages/ui-tree-browser/src/TreeBrowser/v2/TreeCollection/index.tsx | Removes now-dead ESLint disable comments for the a11y role rule. |
| packages/ui-tree-browser/src/TreeBrowser/v1/TreeCollection/index.tsx | Removes now-dead ESLint disable comments for the a11y role rule. |
| packages/ui-scripts/package.json | Replaces eslint peer dependency with oxlint. |
| packages/ui-scripts/lib/commands/lint.ts | Updates ui-scripts lint to invoke oxlint rather than eslint. |
| packages/ui-scripts/lib/commands/index.ts | Updates module import path after moving lint command into lib/commands/. |
| packages/ui-scripts/lib/node_tests/lint.test.ts | Updates tests to assert oxlint invocation and new module path. |
| packages/ui-i18n/src/textDirectionContextConsumer.tsx | Updates an eslint-disable rule id to match the oxlint/react-js rule namespace. |
| packages/ui-color-picker/src/ColorPreset/tests/ColorPreset.test.tsx | Fixes trailing whitespace in a test title string. |
| packages/docs/globals.ts | Updates disable comments to the new oxlint rule namespace for no-relative-imports. |
| package.json | Removes ESLint dependencies and wires oxlint into lint-staged. |
| eslint.config.mjs | Removes the repo’s ESLint flat config (superseded by oxlint configs). |
| .oxlintrc.json | Adds the root oxlint configuration, including JS-plugin wiring and rule mappings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+127
to
+130
| const properImport = path.join( | ||
| importPkg.packageJson.name, | ||
| importBaseName === path.basename(importRoot) ? '' : importBaseName | ||
| ) |
| ImportDeclaration(node) { | ||
| const message = checkImportForRelativePackage( | ||
| node.source.value, | ||
| context.filename |
| const [firstArgument] = node.arguments | ||
| const message = checkImportForRelativePackage( | ||
| firstArgument.value, | ||
| context.filename |
Contributor
Author
HerrTopi
approved these changes
Jul 27, 2026
HerrTopi
left a comment
HerrTopi
left a comment
Contributor
There was a problem hiding this comment.
It seems to be working for vscodium
…older Move lint.ts from lib/test/ into lib/commands/, alongside the other yargs command modules (bump.ts, server.ts, etc.) it is registered with in commands/index.ts. The test/ directory is a leftover name from when ui-scripts absorbed the separate ui-test package.
Add .oxlintrc.json (workspace root and regression-test), porting the ESLint ruleset to oxlint and covering gaps with two custom rules and a jsPlugins bridge: - react-js/* (jsPlugins bridge to eslint-plugin-react): covers no-deprecated, prop-types, no-typos, require-default-props, forbid-foreign-prop-types, missing from oxlint's native react plugin. - instructure/no-relative-imports (custom): blocks relative imports that reach into another package's src, bypassing that package's public API. - notice/notice (custom): requires the MIT license header at the top of source files. - vitest/valid-expect (override, ui-truncate-text only): disabled because oxlint's vitest plugin does not recognize the chai-style assertions used in those tests. Nothing consumes oxlint yet. ESLint still runs via lint-staged, ui-scripts lint, and regression-test's lint script.
Wire oxlint into lint-staged, ui-scripts lint, and regression-test's lint script, replacing ESLint everywhere it ran. Remove ESLint's config files, its now-superseded custom plugin, and every dependency with no remaining consumer: eslint, @eslint/js, @vitest/eslint-plugin, eslint-config-prettier, eslint-import-resolver-node, eslint-module-utils, eslint-plugin-compat, eslint-plugin-jsx-a11y, eslint-plugin-notice, globals, and typescript-eslint. Keep eslint-plugin-react and read-package-up: both are still runtime dependencies of oxlint's jsPlugins bridge and custom rules, respectively.
Two disable comments reference rule names that do not match oxlint's actual rule IDs, so they silently stopped suppressing anything: - packages/__docs__/globals.ts: @instructure/no-relative-imports -> instructure/no-relative-imports. oxlint's custom rule has no @ scope prefix. - packages/ui-i18n/src/textDirectionContextConsumer.tsx: react/forbid-foreign-prop-types -> react-js/forbid-foreign-prop-types. The bridged eslint-plugin-react is aliased react-js. Delete 6 jsx_a11y/role-has-required-aria-props disable comments in TreeBrowser's TreeCollection (v1 and v2): - oxlint's rule does not require aria-selected for the treeitem role at all, unlike e.g. checkbox, which requires aria-checked, so there was nothing for these comments to suppress. oxlint accepts both jsx-a11y/* and jsx_a11y/* spellings for disable comments, so the unrelated jsx-a11y/* comments in Checkbox, FileDrop, Img, NumberInput, and RadioInput needed no change. Also trims a trailing space in one ColorPreset test title, an unrelated vitest/valid-title auto-fix.
Replace the loose mustMatch: 'The MIT License' substring check with an exact match against the full COPYRIGHT_NOTICE text. The old check only looked for that one phrase in the first 1000 characters, so a header with the phrase present but other lines altered, wrong, or missing was accepted as valid. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Correct license headers that drifted from the canonical text and were missed by the old loose oxlint notice check: wrong copyright start years (2018/2021 instead of 2015) in ui-date-time-input, ui-editable, ui-i18n and ui-tree-browser, a "copyrigfht" typo in ui-color-picker, and an extra period after "(MIT)" in vitest.config.mts, vitest.setup.ts, vitest.setup.browser.ts, and a ui-portal test file. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
joyenjoyer
force-pushed
the
instui-5035-oxlint-migration
branch
from
July 28, 2026 12:03
1429aaf to
527cc7d
Compare
Replace expect(...).to.equal(...) chai-style assertions with vitest's native toEqual() in cleanString.test.tsx and cleanData.test.tsx. These were leftover from an old Mocha/Chai migration and were the only reason oxlint's vitest/valid-expect rule was disabled for this package's tests. Remove that now-unneeded per-file override from .oxlintrc.json so the rule applies uniformly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Update the Linters and Code Formatting section to reflect the ESLint to oxlint migration: list the lint/lint:fix/lint:changes commands and swap the ESLint/Stylelint VSCode plugin recommendation for the Oxc extension. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.oxlintrc.json(root +regression-test) porting the existing ESLint ruleset to oxlint's native plugins, its alphajsPluginsbridge for 5eslint-plugin-reactrules oxlint lacks natively, and two custom rules (no-relative-imports,notice) reimplemented for oxlint's JS-plugin APIlint-staged,ui-scripts lint, andregression-test's lint script; remove ESLint's config files, its superseded custom plugin, and 11 now-unused dependencies.eslint-plugin-reactandread-package-upare kept, since both are still runtime deps of oxlint'sjsPluginsbridge and custom rules.eslint-disablecomments whose rule names didn't match oxlint's actual rule IDs and had silently stopped suppressing anything; delete 6 more that are genuinely dead under oxlint'srole-has-required-aria-propsimplementationui-scripts'lint.tsout of a legacylib/test/folder intolib/commands/, alongside the other command modulesTest Plan
tsc -b,vitest, and a full-repo oxlint dry run all confirmed clean (only 5 pre-existing, unrelatedno-undeferrors remain inui-icons's.cjsconfigs, unchanged from before this PR).Fixes INSTUI-5035
🤖 Generated with Claude Code