Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Deduplicate checks between ESLint and TypeScript #3008

Merged
merged 3 commits into from
Sep 18, 2023
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
8 changes: 4 additions & 4 deletions frontend/src/modules/prometheus.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import http from "http"

import Prometheus from "prom-client"
import { register } from "prom-client"
import promBundle from "express-prom-bundle"

import { searchTypes } from "../../src/constants/media"
Expand Down Expand Up @@ -57,7 +57,7 @@ const PrometheusModule: Module = function () {
this.nuxt.hook("close", () => {
metricsServer?.close()
// Clear registry so that metrics can re-register when the server restarts in development
Prometheus.register.clear()
register.clear()
})

this.nuxt.hook("listen", () => {
Expand All @@ -66,9 +66,9 @@ const PrometheusModule: Module = function () {
metricsServer = http
.createServer(async (_, res) => {
res.writeHead(200, {
"Content-Type": Prometheus.register.contentType,
"Content-Type": register.contentType,
})
res.end(await Prometheus.register.metrics())
res.end(await register.metrics())
})
.listen(parseFloat(process.env.METRICS_PORT || "54641"), "0.0.0.0")
})
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/plugins/ua-parse.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import useragent, { Details as UADetails } from "express-useragent"
import { parse, Details as UADetails } from "express-useragent"

import type { Plugin } from "@nuxt/types"

Expand All @@ -12,7 +12,7 @@ const uaParsePlugin: Plugin = (context, inject) => {
}
let ua: UADetails | null
if (typeof userAgent == "string") {
ua = useragent.parse(userAgent)
ua = parse(userAgent)
} else {
ua = null
}
Expand Down
28 changes: 27 additions & 1 deletion packages/eslint-plugin/src/configs/import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,14 @@ import type { TSESLint } from "@typescript-eslint/utils"
* ESLint `import` plugin configuration.
*/
export = {
extends: ["plugin:import/recommended"],
extends: ["plugin:import/recommended", "plugin:import/typescript"],
plugins: ["import"],
rules: {
// `namespace` and `default` are handled by TypeScript
// There's no need to rely on ESLint for this
// https://github.com/import-js/eslint-plugin-import/issues/2878
"import/namespace": "off",
"import/default": "off",
"import/newline-after-import": ["error"],
"import/order": [
"error",
Expand Down Expand Up @@ -65,6 +70,27 @@ export = {
"import/extensions": ["error", "always", { js: "never", ts: "never" }],
},
overrides: [
{
files: ["frontend/**"],
settings: {
"import/resolver": {
typescript: {
project: "frontend/tsconfig.json",
extensions: [".js", ".ts", ".vue", ".png"],
},
},
},
},
{
files: ["packages/**"],
settings: {
"import/resolver": {
typescript: {
project: "packages/*/tsconfig.json",
},
},
},
},
{
files: ["frontend/.storybook/**"],
rules: {
Expand Down
36 changes: 2 additions & 34 deletions packages/eslint-plugin/src/configs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,17 @@ export const project: TSESLint.Linter.Config = {
node: true,
},
parser: "vue-eslint-parser",
parserOptions: {
parser: "@typescript-eslint/parser",
},
extends: [
"eslint:recommended",
"plugin:@typescript-eslint/recommended",
"plugin:eslint-comments/recommended",
"plugin:jsonc/recommended-with-jsonc",
require.resolve("./custom"),
require.resolve("./vue"),
require.resolve("./import"),
require.resolve("./typescript"),
"prettier",
],
plugins: ["@typescript-eslint", "tsdoc", "unicorn"],
plugins: ["unicorn"],
settings: {
"vue-i18n": {
localeDir: "./frontend/src/locales/*.{json}",
Expand All @@ -36,40 +33,12 @@ export const project: TSESLint.Linter.Config = {
semi: ["error", "never"],
"no-console": "off",
"unicorn/filename-case": ["error", { case: "kebabCase" }],
"@typescript-eslint/no-var-requires": ["off"],
},
overrides: [
{
files: ["*.ts"],
rules: {
"tsdoc/syntax": "error",
},
},
{
files: ["*.json", "*.json5", "*.jsonc"],
parser: "jsonc-eslint-parser",
},
{
files: ["frontend/**"],
settings: {
"import/resolver": {
typescript: {
project: "frontend/tsconfig.json",
extensions: [".js", ".ts", ".vue", ".png"],
},
},
},
},
{
files: ["packages/**"],
settings: {
"import/resolver": {
typescript: {
project: "packages/*/tsconfig.json",
},
},
},
},
{
env: { jest: true },
files: ["packages/**/*/test", "frontend/test/unit/**"],
Expand Down Expand Up @@ -128,7 +97,6 @@ export const project: TSESLint.Linter.Config = {
"unicorn/filename-case": "off",
},
},

{
files: ["frontend/src/components/**"],
rules: {
Expand Down
24 changes: 24 additions & 0 deletions packages/eslint-plugin/src/configs/typescript.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import type { TSESLint } from "@typescript-eslint/utils"

export = {
parserOptions: {
parser: "@typescript-eslint/parser",
},
plugins: ["@typescript-eslint", "tsdoc"],
extends: ["plugin:@typescript-eslint/recommended"],
rules: {
"@typescript-eslint/no-var-requires": ["off"],
},
overrides: [
{
files: ["*.ts"],
rules: {
"tsdoc/syntax": "error",
// This rule is disabled above to avoid forcing ESM syntax on regular JS files
Copy link
Contributor

Choose a reason for hiding this comment

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

What does "forcing ESM syntax on regular JS files" mean here? Where do we do that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The disabled rule would raise on every usage of require in our regular JavaScript files and require them to be turned into import statements instead. Our tooling doesn't support that across the board, but our TypeScript files can abide by that rule (because TypeScript supports that syntax out of the box). If we wanted to enable this rule everywhere we would need to change the node scripts we run directly (locales, tapes, etc) to mjs... or run them with ts-node.

// that aren't ready for it yet. We do want to enforce this for TypeScript,
// however, so we re-enable it here.
"@typescript-eslint/no-var-requires": ["error"],
},
},
],
} satisfies TSESLint.Linter.Config
12 changes: 9 additions & 3 deletions tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,18 @@
"esModuleInterop": true,
/* Strict Type-Checking Options */
"strict": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,
"noFallthroughCasesInSwitch": true,
"paths": {
"@openverse/*": ["./packages/*/src"]
}
},

/**
* Disable these in favour of more flexible ESLint rule.
*
* https://typescript-eslint.io/rules/no-unused-vars/#benefits-over-typescript
*/
"noUnusedLocals": false,
"noUnusedParameters": false
}
}