Skip to content

Migrate to ESLint 9 and ESLint flat config #6615

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions .eslintignore

This file was deleted.

55 changes: 0 additions & 55 deletions .eslintrc

This file was deleted.

3 changes: 0 additions & 3 deletions .github/.eslintrc

This file was deleted.

8 changes: 0 additions & 8 deletions dev-server/.eslintrc

This file was deleted.

8 changes: 0 additions & 8 deletions dev-server/ui-playground/.eslintrc

This file was deleted.

6 changes: 0 additions & 6 deletions embedding-examples/.eslintrc

This file was deleted.

88 changes: 88 additions & 0 deletions eslint.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import hypothesis from 'eslint-config-hypothesis';
import jsxA11y from 'eslint-plugin-jsx-a11y';
import globals from 'globals';
import tseslint from 'typescript-eslint';

export default tseslint.config(
{
ignores: [
'.tox/**/*',
'.yalc/**/*',
'.yarn/**/*',
'build/**/*',
'**/vendor/**/*.js',
'**/coverage/**/*',
'docs/_build/*',
'dev-server/static/**/*.js',
],
Copy link
Member

Choose a reason for hiding this comment

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

Many of these files are already ignored by .gitignore. Does ESLint not automatically respect that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, doesn't seem to. I even had to add a few more there that were not part of .eslintignore and were getting scanned now (.tox, .yarn and .yalc specifically).

},
...hypothesis,
...tseslint.configs.recommended,
jsxA11y.flatConfigs.recommended,
{
rules: {
'prefer-arrow-callback': [
'error',
{
allowNamedFunctions: true,
},
],

'object-shorthand': ['error', 'properties'],
'react/prop-types': 'off',
'@typescript-eslint/no-unused-vars': 'error',
'no-use-before-define': 'off',

'@typescript-eslint/no-use-before-define': [
'error',
{
functions: false,
typedefs: false,
ignoreTypeReferences: false,
},
],

'@typescript-eslint/ban-ts-comment': 'off',
'@typescript-eslint/no-empty-function': 'off',
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
'@typescript-eslint/no-this-alias': 'off',
'@typescript-eslint/consistent-type-assertions': 'error',
'@typescript-eslint/consistent-type-imports': 'error',
Copy link
Member

Choose a reason for hiding this comment

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

I think we use the same rules in other TS-enabled projects. It would be good to centralize them in eslint-config-hypothesis where appropriate.

},
},

// Annotator module
{
files: ['src/annotator/**/*.{js|tx|tsx}'],
rules: {
'no-restricted-properties': [
2,
{
// Disable `bind` usage in annotator/ code to prevent unexpected behavior
// due to broken bind polyfills. See
// https://github.com/hypothesis/client/issues/245
property: 'bind',
message:
'Use function expressions instead of bind() in annotator/ code',
},
],
},
},

// Scripts and configuration files
{
files: ['**/*.js'],
Copy link
Member

Choose a reason for hiding this comment

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

I think I would prefer to use an allow-list rather than making these rules apply to all files and then attempting to exclude src/. It depends how cumbersome maintaining the allow-list is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, but for now let's just migrate the config as-is (as much as possible). We can adjust that later.

Copy link
Member

Choose a reason for hiding this comment

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

OK

ignores: ['src/**'],
rules: {
'@typescript-eslint/no-var-requires': 'off',
'no-console': 'off',
'react-hooks/rules-of-hooks': 'off',
},
languageOptions: {
globals: {
...globals.node,
},
},
},
);
16 changes: 8 additions & 8 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@
"@types/shallowequal": "^1.1.1",
"@types/showdown": "^2.0.0",
"@types/sinon": "^17.0.3",
"@typescript-eslint/eslint-plugin": "^7.0.1",
"@typescript-eslint/parser": "^7.0.1",
"approx-string-match": "^2.0.0",
"autoprefixer": "^10.0.1",
"axe-core": "^4.0.0",
Expand All @@ -58,16 +56,17 @@
"enzyme-adapter-preact-pure": "^4.0.1",
"escape-html": "^1.0.3",
"escape-string-regexp": "^4.0.0",
"eslint": "^8.3.0",
"eslint-config-hypothesis": "^2.6.0",
"eslint-plugin-jsx-a11y": "^6.2.3",
"eslint-plugin-mocha": "^10.0.1",
"eslint-plugin-react": "^7.12.4",
"eslint-plugin-react-hooks": "^4.0.4",
"eslint": "^9.12.0",
"eslint-config-hypothesis": "^3.0.0",
"eslint-plugin-jsx-a11y": "^6.10.0",
"eslint-plugin-mocha": "^10.5.0",
"eslint-plugin-react": "^7.37.1",
"eslint-plugin-react-hooks": "^5.0.0",
"express": "^5.0.1",
"fancy-log": "^2.0.0",
"fetch-mock": "11",
"focus-visible": "^5.0.0",
"globals": "^15.11.0",
"gulp": "^5.0.0",
"gulp-changed": "^5.0.1",
"hammerjs": "^2.0.4",
Expand Down Expand Up @@ -101,6 +100,7 @@
"tailwindcss": "^3.0.2",
"tiny-emitter": "^2.0.2",
"typescript": "^5.0.2",
"typescript-eslint": "^8.9.0",
"wrap-text": "^1.0.7"
},
"browserslist": "chrome 92, firefox 90, safari 14.1",
Expand Down
10 changes: 0 additions & 10 deletions scripts/.eslintrc

This file was deleted.

18 changes: 0 additions & 18 deletions src/annotator/.eslintrc.cjs

This file was deleted.

2 changes: 1 addition & 1 deletion src/annotator/anchoring/html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export function describe(root: Element, range: Range) {
if (anchor) {
result.push(anchor.toSelector());
}
} catch (error) {
} catch {
// If resolving some anchor fails, we just want to skip it silently
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/anchoring/xpath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export function nodeFromXPath(
): Node | null {
try {
return evaluateSimpleXPath(xpath, root);
} catch (err) {
} catch {
return document.evaluate(
'.' + xpath,
root,
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/config/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export function settingsFrom(window_: Window): SettingsGetters {
if (queryFragmentMatch) {
try {
return decodeURIComponent(queryFragmentMatch[2]);
} catch (err) {
} catch {
// URI Error should return the page unfiltered.
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/frame-observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export class FrameObserver {
this._removeFrame(frame);
});
this._onFrameAdded(frame);
} catch (e) {
} catch {
console.warn(
`Unable to inject the Hypothesis client (from '${document.location.href}' into a cross-origin frame '${frame.src}')`,
);
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/guest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,7 @@ export class Guest extends TinyEmitter implements Annotator, Destroyable {
// this anchor.
const textRange = TextRange.fromRange(range);
anchor = { annotation, target, range: textRange };
} catch (err) {
} catch {
anchor = { annotation, target };
}
return anchor;
Expand Down
6 changes: 3 additions & 3 deletions src/annotator/integrations/html-metadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ export class HTMLMetadata {
try {
const href = this._absoluteUrl(link.href);
links.push({ href, rel: link.rel, type: link.type });
} catch (e) {
} catch {
// Ignore URIs which cannot be parsed.
}
}
Expand All @@ -197,7 +197,7 @@ export class HTMLMetadata {
href: this._absoluteUrl(url),
type: 'application/pdf',
});
} catch (e) {
} catch {
// Ignore URIs which cannot be parsed.
}
}
Expand Down Expand Up @@ -253,7 +253,7 @@ export class HTMLMetadata {
if (['shortcut icon', 'icon'].includes(link.rel)) {
try {
favicon = this._absoluteUrl(link.href);
} catch (e) {
} catch {
// Ignore URIs which cannot be parsed.
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/integrations/pdf.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ export class PDFIntegration extends TinyEmitter implements Integration {
// Wait for PDF to load.
try {
await this.uri();
} catch (e) {
} catch {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/annotator/integrations/test/html-metadata-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,9 +342,9 @@ describe('HTMLMetadata', () => {
// location in tests, create a proxy object in front of our blank HTML
// document.
const fakeDocument = {
createElement: htmlDoc.createElement.bind(htmlDoc), // eslint-disable-line no-restricted-properties
createElement: htmlDoc.createElement.bind(htmlDoc),
baseURI: baseURI ?? href,
querySelectorAll: htmlDoc.querySelectorAll.bind(htmlDoc), // eslint-disable-line no-restricted-properties
querySelectorAll: htmlDoc.querySelectorAll.bind(htmlDoc),
location: {
href,
},
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/range-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export function isNodeInRange(range: Range, node: Node) {
// Check end of node is after start of range.
range.comparePoint(node, length) >= 0
);
} catch (e) {
} catch {
// `comparePoint` may fail if the `range` and `node` do not share a common
// ancestor or `node` is a doctype.
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ export class Sidebar implements Destroyable {

// Suppressing ban-types here because the functions are originally defined
// as `Function` somewhere else. To be fixed when that is migrated to TS
// eslint-disable-next-line @typescript-eslint/ban-types
// eslint-disable-next-line @typescript-eslint/no-unsafe-function-type
const eventHandlers: Array<[SidebarToHostEvent, Function | undefined]> = [
['loginRequested', this.onLoginRequest],
['logoutRequested', this.onLogoutRequest],
Expand Down
2 changes: 1 addition & 1 deletion src/annotator/util/emitter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable @typescript-eslint/ban-types */
/* eslint-disable @typescript-eslint/no-unsafe-function-type */

/*
* Disable @typescript-eslint/ban-types for the whole file, as changing the
Expand Down
2 changes: 1 addition & 1 deletion src/boot/browser-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function isBrowserSupported(): boolean {

try {
return checks.every(check => check());
} catch (err) {
} catch {
return false;
}
}
4 changes: 2 additions & 2 deletions src/karma.config.cjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* global __dirname */
/* global __dirname require module */

// eslint-disable-next-line @typescript-eslint/no-var-requires
// eslint-disable-next-line @typescript-eslint/no-require-imports
const path = require('path');

module.exports = function (config) {
Expand Down
1 change: 0 additions & 1 deletion src/shared/listener-collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export class ListenerCollection {
this._listeners.set(symbol, {
eventTarget,
eventType,
// eslint-disable-next-line object-shorthand
listener: listener as EventListener,
options,
});
Expand Down
Loading