Lint without building cljs (#83718)
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
## Linting and Formatting
|
||||
|
||||
- **Lint:** `bun run lint-oxlint-pure`
|
||||
- **Lint:** `bun run lint-oxlint`
|
||||
- Run oxlint on the codebase
|
||||
- **Format:** `bun run format`
|
||||
- Format code with oxfmt (`oxfmt --write`); use `bun run lint-format-pure` to check without writing
|
||||
|
||||
@@ -39,7 +39,7 @@ what and who may depend on whom.**
|
||||
| module elements, tiers, `enforceSharedTiers`, `enforcePublicApi` | `frontend/lint/module-boundaries.mjs` |
|
||||
| shared sub-tiers and levels, cluster rules | `frontend/lint/shared-tiers.mjs` |
|
||||
| standalone boundaries lint (all violations, incl. grandfathered) | `bun run module-boundaries` (config `eslint.config.module-boundaries.mjs`) |
|
||||
| PR lint (only enforced modules fail CI) | `bun run lint-oxlint-pure` |
|
||||
| PR lint (only enforced modules fail CI) | `bun run lint-oxlint` |
|
||||
| side-effect-free directories and their exceptions | `frontend/build/shared/rspack/side-effect-free-modules.js` (`SIDE_EFFECT_FREE_PATHS`, `SIDE_EFFECT_PATHS`) |
|
||||
| the lint rules that enforce that promise | `metabase/no-module-side-effects`, `metabase/no-base-api-access` in `frontend/lint/eslint-plugin-metabase/rules/` |
|
||||
| public-api enforcement | `metabase/enforce-module-public-api` rule, driven by `getPublicApiModules()` |
|
||||
@@ -199,7 +199,7 @@ prefix; never generic `-shared` / `-feature` suffixes.
|
||||
```
|
||||
grep -rn "<old path>" frontend enterprise/frontend e2e # must be empty
|
||||
bunx eslint --no-warn-ignored <touched files>
|
||||
bun run lint-oxlint-pure # enforce-module-public-api included
|
||||
bun run lint-oxlint # enforce-module-public-api included
|
||||
bun run type-check-pure
|
||||
bun run test-unit-keep-cljs <touched folders' specs>
|
||||
bun run module-boundaries 2>&1 | tail -1 # after; no violation may name the new files
|
||||
|
||||
@@ -172,7 +172,7 @@ export const OverviewStory = {
|
||||
- **Scope controls per story.** A static matrix should hide knobs it ignores: `parameters.controls = { include: [...] }` (or `{ disable: true }`). Wire any kept knob (e.g. `children`) through to the cells so it does something.
|
||||
- `loki.config.js` has a `storiesFilter`; check it before renaming a story that's already covered.
|
||||
|
||||
Validate: `bun run lint-oxlint-pure -- <files>` and `bun run type-check-pure`.
|
||||
Validate: `bun run lint-oxlint -- <files>` and `bun run type-check-pure`.
|
||||
|
||||
### Optional: verify in the browser yourself
|
||||
|
||||
@@ -217,7 +217,7 @@ General gotchas when extending Mantine components — adapt per component:
|
||||
- **Built-in icons use `currentColor`**, so they inherit the slot's text color.
|
||||
- For text on a brand fill, follow the existing convention (`text-primary-inverse`) unless the user wants pure white; flag the dark-theme implication.
|
||||
|
||||
After edits: `npx stylelint <css>`, `LINT_CSS_MODULES=true bun run lint-oxlint-pure -- <files>`, `bun run type-check-pure`.
|
||||
After edits: `npx stylelint <css>`, `LINT_CSS_MODULES=true bun run lint-oxlint -- <files>`, `bun run type-check-pure`.
|
||||
|
||||
## Phase 5 — Iterate with the user
|
||||
|
||||
|
||||
6
.github/workflows/frontend.yml
vendored
6
.github/workflows/frontend.yml
vendored
@@ -38,17 +38,13 @@ jobs:
|
||||
- uses: actions/checkout@v7
|
||||
- name: Prepare front-end environment
|
||||
uses: ./.github/actions/prepare-frontend
|
||||
- name: Prepare back-end environment
|
||||
uses: ./.github/actions/prepare-backend
|
||||
|
||||
- name: Compile CLJS
|
||||
run: bun run build-pure:cljs
|
||||
- name: Find CSS variables that are used but never defined
|
||||
run: bun run find-never-defined-css-variables
|
||||
- name: Run oxfmt formatting linter
|
||||
run: bun run lint-format-pure
|
||||
- name: Run oxlint linter (including module boundaries)
|
||||
run: bun run lint-oxlint-pure
|
||||
run: bun run lint-oxlint
|
||||
- name: Verify lint configuration and behavior
|
||||
run: bun run test-oxlint
|
||||
- name: Run CSS linter
|
||||
|
||||
@@ -178,7 +178,7 @@ The first rule of frontend style, is we want to avoid talking about frontend sty
|
||||
|
||||
We use [oxfmt](https://oxc.rs/) to format our JavaScript and TypeScript code, and it is enforced by CI. We recommend setting your editor to "format on save". You can also format code using `bun run format`, and verify it has been formatted correctly using `bun run lint-format-pure`. Oxfmt also sorts imports, so `bun run format` fixes import order too.
|
||||
|
||||
We use [oxlint](https://oxc.rs/) to enforce JavaScript and TypeScript lint rules, including module boundaries. Run `bun run lint-oxlint` to lint, or `bun run lint-oxlint-fix` to apply lint fixes. Both commands build the CLJS output first. If you've already built it, `bun run lint-oxlint-pure` skips that step. For how the lint configuration works, see `frontend/lint/OXLINT.md`.
|
||||
We use [oxlint](https://oxc.rs/) to enforce JavaScript and TypeScript lint rules, including module boundaries. Run `bun run lint-oxlint` to lint, or `bun run lint-oxlint-fix` to apply lint fixes. For how the lint configuration works, see `frontend/lint/OXLINT.md`.
|
||||
|
||||
### Miscellaneous notes on coding style
|
||||
|
||||
|
||||
@@ -483,7 +483,7 @@ const configs = [
|
||||
],
|
||||
"import/no-unresolved": [
|
||||
"error",
|
||||
{ ignore: ["@metabase/embedding-sdk-react"] },
|
||||
{ ignore: ["@metabase/embedding-sdk-react", "^cljs/"] },
|
||||
],
|
||||
"metabase/no-direct-helper-import": "error",
|
||||
"metabase/no-unsafe-element-filtering": "warn",
|
||||
|
||||
35
frontend/lint/scripts/run-oxlint.mjs
Normal file
35
frontend/lint/scripts/run-oxlint.mjs
Normal file
@@ -0,0 +1,35 @@
|
||||
import { spawnSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const FLAGS = [
|
||||
"--disable-nested-config",
|
||||
"--max-warnings",
|
||||
"0",
|
||||
"--report-unused-disable-directives",
|
||||
];
|
||||
const DEFAULT_PATHS = ["enterprise/frontend", "frontend", "e2e"];
|
||||
|
||||
export function getOxlintArgs(args, pathExists) {
|
||||
const hasPath = args.some(
|
||||
(arg) => !arg.startsWith("-") && (pathExists(arg) || /[/.]/.test(arg)),
|
||||
);
|
||||
return [...FLAGS, ...args, ...(hasPath ? [] : DEFAULT_PATHS)];
|
||||
}
|
||||
|
||||
if (process.argv[1] === fileURLToPath(import.meta.url)) {
|
||||
const oxlint = path.resolve(
|
||||
import.meta.dirname,
|
||||
"../../../node_modules/.bin/oxlint",
|
||||
);
|
||||
const result = spawnSync(
|
||||
oxlint,
|
||||
getOxlintArgs(process.argv.slice(2), fs.existsSync),
|
||||
{ stdio: "inherit" },
|
||||
);
|
||||
if (result.error) {
|
||||
throw result.error;
|
||||
}
|
||||
process.exit(result.status ?? 1);
|
||||
}
|
||||
64
frontend/lint/tests/oxlint-run.test.mjs
Normal file
64
frontend/lint/tests/oxlint-run.test.mjs
Normal file
@@ -0,0 +1,64 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { getOxlintArgs } from "../scripts/run-oxlint.mjs";
|
||||
|
||||
const flags = [
|
||||
"--disable-nested-config",
|
||||
"--max-warnings",
|
||||
"0",
|
||||
"--report-unused-disable-directives",
|
||||
];
|
||||
const defaultPaths = ["enterprise/frontend", "frontend", "e2e"];
|
||||
const file = "frontend/src/metabase/dev.ts";
|
||||
const pathExists = (arg) => arg === file;
|
||||
|
||||
test("should lint the default paths when no arguments are given", () => {
|
||||
assert.deepEqual(getOxlintArgs([], pathExists), [...flags, ...defaultPaths]);
|
||||
});
|
||||
|
||||
test("should lint the default paths when only --fix is given", () => {
|
||||
assert.deepEqual(
|
||||
getOxlintArgs(["--fix"], () => true),
|
||||
[...flags, "--fix", ...defaultPaths],
|
||||
);
|
||||
});
|
||||
|
||||
test("should lint only an existing file when one is given", () => {
|
||||
assert.deepEqual(getOxlintArgs([file], pathExists), [...flags, file]);
|
||||
});
|
||||
|
||||
test("should lint the default paths when a flag value is not an existing path", () => {
|
||||
assert.deepEqual(getOxlintArgs(["--format", "json"], pathExists), [
|
||||
...flags,
|
||||
"--format",
|
||||
"json",
|
||||
...defaultPaths,
|
||||
]);
|
||||
});
|
||||
|
||||
test("should lint only an existing file when it is given with --fix", () => {
|
||||
assert.deepEqual(getOxlintArgs(["--fix", file], pathExists), [
|
||||
...flags,
|
||||
"--fix",
|
||||
file,
|
||||
]);
|
||||
});
|
||||
|
||||
test("should pass a missing path to oxlint without the default paths", () => {
|
||||
assert.deepEqual(getOxlintArgs(["frontend/src/nope.ts"], pathExists), [
|
||||
...flags,
|
||||
"frontend/src/nope.ts",
|
||||
]);
|
||||
});
|
||||
|
||||
test("should pass a missing file name to oxlint without the default paths", () => {
|
||||
assert.deepEqual(getOxlintArgs(["dev.ts"], pathExists), [...flags, "dev.ts"]);
|
||||
});
|
||||
|
||||
test("should lint only an existing directory when one is given", () => {
|
||||
assert.deepEqual(
|
||||
getOxlintArgs(["frontend"], (arg) => arg === "frontend"),
|
||||
[...flags, "frontend"],
|
||||
);
|
||||
});
|
||||
@@ -440,7 +440,7 @@
|
||||
"chromatic": "bun run build:cljs && NODE_OPTIONS=--max-old-space-size=8192 chromatic",
|
||||
"ci": "bun run ci-frontend && bun run ci-backend",
|
||||
"ci-backend": "clojure -X:dev:ee:ee-dev:drivers:drivers-dev:eastwood && clojure -X:dev:test",
|
||||
"ci-frontend": "bun run lint && bun run test",
|
||||
"ci-frontend": "bun run lint && bun run type-check && bun run test",
|
||||
"clean-cypress-artifacts": "rm -R -f cypress/",
|
||||
"clean-dev:cljs": "rm -rf target/cljs_dev/*",
|
||||
"clean-dev:js": "rm -rf resources/frontend_client/app/dist target/classes/frontend_client/app/dist resources/frontend_client/app/embedding-sdk target/classes/frontend_client/app/embedding-sdk",
|
||||
@@ -473,11 +473,10 @@
|
||||
"generate-cypress-html-report": "mochawesome-merge cypress/reports/mochareports/*.json > cypress/reports/cypress-test-report.json && marge cypress/reports/cypress-test-report.json -o cypress/reports --inline",
|
||||
"generate-openapi": "clojure -M:run:ee generate-openapi-spec",
|
||||
"lint-openapi": "redocly lint resources/openapi/openapi.json --skip-rule security-defined --skip-rule operation-operationId --skip-rule no-unused-components --skip-rule info-license --skip-rule no-path-trailing-slash --skip-rule no-identical-paths --skip-rule no-ambiguous-paths --skip-rule path-parameters-defined --skip-rule spec-components-invalid-map-name",
|
||||
"lint": "bun run lint-oxlint && bun run lint-format && bun run lint-docs-links && bun run lint-yaml && bun run type-check",
|
||||
"lint": "bun run lint-oxlint && bun run lint-format && bun run lint-docs-links && bun run lint-yaml",
|
||||
"lint-docs-links": "bun install && ./bin/verify-doc-links",
|
||||
"lint-css": "stylelint './frontend/**/*.css'",
|
||||
"lint-oxlint": "bun run build:cljs && bun run lint-oxlint-pure",
|
||||
"lint-oxlint-pure": "oxlint --disable-nested-config --max-warnings 0 --report-unused-disable-directives enterprise/frontend frontend e2e",
|
||||
"lint-oxlint": "node frontend/lint/scripts/run-oxlint.mjs",
|
||||
"test-oxlint": "node --test frontend/lint/tests/oxlint-*.test.mjs",
|
||||
"lint-config-update": "node frontend/lint/update-recommended-rules.mjs && oxfmt frontend/lint/recommended-rules.json frontend/lint/oxlint/rule-defaults.json",
|
||||
"module-boundaries": "eslint --config eslint.config.module-boundaries.mjs 'frontend/src/**/*.{ts,tsx}' 'enterprise/frontend/src/**/*.{ts,tsx}'",
|
||||
|
||||
Reference in New Issue
Block a user