From a4a2735b5464b3b1214a27c4dbc286c32a245e02 Mon Sep 17 00:00:00 2001 From: DanMat Date: Sun, 26 Jul 2026 15:09:59 -0400 Subject: [PATCH] chore: lint Packkit's own source (eat our own cooking) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Packkit generated publint, strict TS, and lint gates for other people's packages but ran none on itself. It now lints its own source with the same stack it generates for users — eslint flat config + @eslint/js — via `npm run lint`, a `check` script (lint + test), and a CI lint job. It immediately paid off, catching real issues on the first run: - a duplicate `minify` key in the CLI arg parser (src/cli/args.js) — a genuine defect the parser silently tolerated; - dead vars/imports: an always-empty `expectApi`, an unused `toJson` import, an unused `staged` param threaded into lefthookYml, and leftover `install`/`run` locals in the monorepo generators. All fixed; generation output is unchanged (verified) and `--minify` still parses. eslint is clean, 87 tests pass. Full `tsc --checkJs` was assessed and deferred — 95 errors, almost all the "incrementally-built object literal" pattern (noise) rather than real bugs. Noted in the roadmap. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/ci.yml | 10 + ROADMAP.md | 2 +- docs/packkit-core.js | 7 +- eslint.config.js | 36 ++ package-lock.json | 905 +++++++++++++++++++++++++++++++++- package.json | 7 +- src/cli/args.js | 1 - src/core/features/githooks.js | 4 +- src/core/features/test.js | 2 - src/core/monorepo.js | 2 - test/embedded.test.js | 3 +- 11 files changed, 962 insertions(+), 17 deletions(-) create mode 100644 eslint.config.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4c14921..17b71fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,6 +4,16 @@ on: branches: [main] pull_request: jobs: + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-node@v7 + with: + node-version: '24' + cache: npm + - run: npm ci + - run: npm run lint test: runs-on: ubuntu-latest strategy: diff --git a/ROADMAP.md b/ROADMAP.md index 5757d1b..90a0ec0 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -96,7 +96,7 @@ _Second external review, this one of the codebase rather than the experience. Ra **The theme worth acting on: Packkit doesn't eat its own cooking.** It generates `publint` + are-the-types-wrong checks, strict TypeScript, and lint gates for other people's packages, then ships without any of them. `npm run check:pkg` is something Packkit writes for you, not something it runs on itself — and its own package would fail it. - [ ] **Type declarations for the public API** — `exports` exposes `.`, `./core`, `./cli`, `./scaffold`, with **no `types`**. Programmatic consumers and the MCP server get nothing. JSDoc + `tsc --declaration` is enough; a rewrite isn't needed. Types wanted for `PackkitConfig`, the resolved config, `GenerateResult`, the file map, preset names, and scaffold results. **v3 requirement.** -- [ ] **Self-enforcement** — root scripts are `start, test, check:deps, integration, build:web, update:node, gen:reference, sync:mcp`. No lint, no format check, no typecheck. Add a `check` script running all of them, plus `tsc --allowJs --checkJs --noEmit` to catch API drift even while the source stays JS. +- [x] ~~**Self-enforcement**~~ — **Shipped (3.1).** Packkit now lints its own source with the stack it generates for users (eslint flat config + `@eslint/js`): `npm run lint`, a `check` script (lint + test), and a CI lint job. It immediately earned its keep — caught a real duplicate `minify` key in the CLI arg parser plus several dead vars/imports, all fixed. Full `tsc --checkJs` was assessed and deferred: 95 errors, nearly all the incremental-object-literal pattern (noise), not worth annotating around right now — though it did surface the dead `reactEntry` reference removed in the CLI-on-embedded PR. - [ ] **Validate generated paths at the writer** — `writeProject` joins each relative path onto the target and writes it. Fine while every feature is trusted first-party code; a security boundary the moment third-party features, community recipes, or MCP-supplied data can contribute paths. Reject absolute paths, `..` escapes, and post-normalization collisions **in the writer**, not per feature — before extensibility ships, not after. - [ ] **Collision diagnostics** — `deepMerge` ends in `return source`, so when two features set the same `scripts.build` or `exports` key the last one silently wins. Additive fields (dependencies) should keep merging; `scripts`, `exports`, `bin`, `files` should report which features collided. Same for two features emitting the same file path. - [ ] **Structured subprocess errors** — `run()` returns a boolean and discards stdout/stderr when quiet, so "install failed" can't distinguish a missing executable from a network error, an unconfigured git identity, or a rejected push. Return `{ ok, command, exitCode, stdout, stderr, category }`; the CLI can still print something friendly, while MCP callers get something actionable. diff --git a/docs/packkit-core.js b/docs/packkit-core.js index 9aa07b5..f52ed4b 100644 --- a/docs/packkit-core.js +++ b/docs/packkit-core.js @@ -1303,7 +1303,6 @@ import { app } from './app.js';`; } const api = runner === "jest" ? `` : `import { describe, it, expect } from 'vitest'; `; - const expectApi = runner === "jest" ? "" : ""; return [ api + `import { greet } from '${imp}';`, ``, @@ -1471,7 +1470,7 @@ var githooks_default = { pkg.scripts.prepare = "husky"; pkg.devDependencies.husky = "^9.1.0"; } else if (cfg.gitHooks === "lefthook") { - files["lefthook.yml"] = lefthookYml(cfg, staged); + files["lefthook.yml"] = lefthookYml(cfg); pkg.scripts.prepare = "lefthook install || true"; pkg.devDependencies.lefthook = "^2.0.0"; } @@ -1482,7 +1481,7 @@ var githooks_default = { return { files, pkg }; } }; -function lefthookYml(cfg, staged) { +function lefthookYml(cfg) { const cmd = cfg.lint === "biome" ? "biome check --write --no-errors-on-unmatched {staged_files}" : cfg.lint === "none" ? "npm test" : "prettier --write {staged_files}"; return [ "pre-commit:", @@ -2498,7 +2497,6 @@ function buildMonorepo(cfg) { test: exampleTest2(`import { shout } from './index.js';`, `expect(shout('world')).toBe('HELLO, WORLD!')`), deps: { [core]: wsProto } }); - const install = pm === "npm" ? "npm install" : `${pm} install`; return { config: cfg, files, @@ -2517,7 +2515,6 @@ function buildFullstack(cfg) { const scope = cfg.name.replace(/^@/, "").split("/")[0]; const shared = `@${scope}/shared`; const wsProto = pm === "pnpm" ? "workspace:*" : "*"; - const run2 = (s) => pm === "npm" ? `npm run ${s}` : `${pm} ${s}`; for (const feat of [community_default, agents_default, gitfiles_default]) { if (feat.active(cfg)) Object.assign(files, feat.apply(cfg).files); } diff --git a/eslint.config.js b/eslint.config.js new file mode 100644 index 0000000..d362008 --- /dev/null +++ b/eslint.config.js @@ -0,0 +1,36 @@ +// Packkit lints its own source with the same stack it generates for users +// (eslint flat config + @eslint/js recommended). Kept deliberately lean: this +// is here to catch real mistakes — undefined names, unreachable code, unused +// values — not to enforce a style (formatting isn't wired up yet). + +import js from '@eslint/js'; +import globals from 'globals'; + +export default [ + { + ignores: ['docs/packkit-core.js', 'node_modules/**', 'coverage/**'], + }, + js.configs.recommended, + { + files: ['**/*.js', '**/*.mjs'], + languageOptions: { + ecmaVersion: 2023, + sourceType: 'module', + globals: { ...globals.node }, + }, + rules: { + // Flag unused values, but let intentionally-ignored args (prefixed _) pass. + 'no-unused-vars': ['error', { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }], + 'no-undef': 'error', + 'no-constant-condition': ['error', { checkLoops: false }], + }, + }, + { + // The web configurator runs in the browser, so it has DOM globals, not Node. + // JSZip is loaded from a