Comfy-Org / Comfy-Org/ComfyUI_frontend

apps/website is not covered by pnpm lint — violations reach main and ambush the next person to merge

Open
#14,788 0 comments 0 reactions 0 assignees View on GitHub
area:CI/CD
Dominant language
TypeScript
Stars
2k
Forks
699
Avg merge
1d 7h
Merged PRs (30d)
490

Description

`apps/website` is linted by the pre-commit hook and by nothing else.

The root script is:

```json
"lint": "pnpm stylelint && oxlint src browser_tests --type-aware && eslint src --cache"
```

`apps/website` appears in neither the `oxlint` nor the `eslint` argument list, and `ci-lint-format.yaml` runs that same script. `lint-staged` does lint it, but only over files a commit happens to stage — so a violation reaches `main` whenever it is introduced in a commit that does not stage it, or by anyone whose hook did not run.

The failure mode is delayed and lands on the wrong person. `apps/website/src/templates/model-launch/modelLaunchPages.test.ts` on `main` today:

```
apps/website/src/templates/model-launch/modelLaunchPages.test.ts:15:10:
error vitest(consistent-each-for): `describe` can not be used with `.each` ...
```

Merging `main` into any branch stages every merged file, lint-staged lints all of them, and the merge is blocked by a file the author never touched. Hit while merging `main` into #14484; fixed there as a drive-by (`describe.each` → `describe.for`).

Two things to decide:

1. Add `apps/website` to the root `lint` script so CI covers it. Currently clean apart from the item above plus three warnings (`valid-expect` ×2, `triple-slash-reference` ×1), so the cost of turning it on is small — and it will only grow.
2. The same question applies to the other workspace packages that the root script omits.

Separately, `lint-staged` on a large merge invoked `oxlint --type-aware` over ~600 files and was `SIGKILL`ed under the default heap; it succeeded with `NODE_OPTIONS=--max-old-space-size=8192`. Worth a cap or a chunk size so merge commits do not OOM.

Contributor guide

Open the contributing guide

Research direction

Start with the root package.json lint script and ci-lint-format.yaml, then inspect lint-staged configuration and the omitted workspace packages. Reproduce the current coverage gap with apps/website/src/templates/model-launch/modelLaunchPages.test.ts and assess the reported oxlint SIGKILL case. Done means the agreed workspace lint scope is covered in CI without introducing merge-time failures or OOMs.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
ci-cd, tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.