Make npm run lint work in code-reference-finder - #260
Open
Hotragn wants to merge 2 commits into
Open
Conversation
…entry These were the only two recipes with a package.json and no lockfile, so npm ci cannot run in either and there is nothing pinned: every install re-resolves every caret range, and OSV-Scanner has no lock to read, which means neither recipe's dependency tree is covered by the repo's own vulnerability scanning. Checked first that the absence looks accidental rather than deliberate. Neither declares a packageManager, neither has a yarn/pnpm/bun lockfile, and neither .gitignore mentions lockfiles -- both ignore only node_modules. The other 30 recipes all commit one. Generated with npm install --package-lock-only, so no source and no package.json changes. lockfileVersion 3 in both, matching all 32 existing lockfiles. Both recipes declare "@tiny-fish/sdk": "latest", which the lock now pins to 0.4.1. That is what an install already produced, so it is not a behaviour change, and it matches how the repo already works: 17 recipes declare latest and commit a lockfile pinning it. Verified from scratch in both: rm -rf node_modules && npm ci exits 0, and tsc --noEmit exits 0.
The eighth and last recipe from tinyfish-io#248. npm run lint fails because this is Next 16.1.6 with "lint": "next lint", and next lint was removed in Next 16 -- the CLI reads "lint" as a directory argument: Invalid project directory provided, no such directory: .../code-reference-finder/lint This was the emptiest of the eight: no eslint.config.*, no eslint and no eslint-config-next. fast-qa in tinyfish-io#258 at least had a config file that could not resolve; here there was nothing at all. Adds eslint ^9 and eslint-config-next 16.1.6 as devDependencies, matching pharmacy-panic, which is also on Next 16.1.6 and pins the same version, plus an eslint.config.mjs identical to the one the ten already-working Next 16 recipes use. Script moves to eslint. The first lint run surfaced one error, in LiveBrowserPreview.tsx: an effect resetting an iframe loading flag whenever streamingUrl changed, tripping react-hooks/set-state-in-effect. Same code and same fix as tenders-finder in tinyfish-io#250 -- track which URL actually loaded instead of a boolean, so a new URL restores the spinner with no effect at all. Fixed identically so the two recipes do not diverge on the same component. useEffect was the file's only use, so the import goes too. Left the '_analysis is defined but never used' warning in lib/search.ts alone: it is a warning rather than an error, and the underscore prefix reads as a deliberate unused binding. Verified from scratch: rm -rf node_modules && npm ci, then npm run lint and tsc --noEmit, all exit 0. Stacked on tinyfish-io#259, which introduces this recipe's lockfile -- adding ESLint has to rewrite it, so the two cannot be independent without conflicting.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The 8th and last recipe from #248.
Stacked on #259 — it includes that PR's commit, because #259 creates
code-reference-finder/package-lock.jsonand adding ESLint has to rewrite it. Keeping them separate would have meant two PRs both touching a file that doesn't exist onmainyet, and a guaranteed conflict for whichever merged second. Merge #259 first and this rebases to just the lint change; or merge this one and you get both.The bug
Next 16.1.6 with
"lint": "next lint", andnext lintwas removed in Next 16 — the CLI readslintas a directory argument.This one was the emptiest of the eight: no
eslint.config.*, noeslint, noeslint-config-next.fast-qain #258 at least had a config file to resolve; here there was nothing.The fix
eslint: ^9andeslint-config-next: 16.1.6as devDependencies, matchingpharmacy-panic— also on Next 16.1.6, same pin.eslint.config.mjsin the subpath style used by all ten already-working Next 16 recipes, byte-identical to theirs.eslint.The one error its first lint run surfaced
src/components/LiveBrowserPreview.tsx:22—react-hooks/set-state-in-effect, and the same pattern astenders-finderin #250:Tracking which URL loaded instead of a boolean means a new
streamingUrlbrings the spinner back on its own, with no effect and no synchronising render. Fixed the same way here as in #250 so the two recipes don't diverge on identical code.useEffectwas the file's only use, so the import goes too.Left alone
src/lib/search.ts:13—'_analysis' is defined but never used, a warning, not an error. The underscore prefix is the conventional marker for a deliberately unused binding, so I read it as intentional and didn't touch it. Lint exits 0 with it present.Verification
Lockfile grows by ~370 entries — the ESLint toolchain, same as #258. That delta sits on top of the lockfile #259 introduces.
#248 is now complete
stay-scout-hubrestaurant-comparison-tool,tutor-finder,scholarship-finder,summer-school-finder,tenders-finderfast-qacode-reference-finderAll eight verified the same way:
npm run lintandtsc --noEmitexit 0 from a cleannpm ci.research-sentryremains the only recipe with nolintscript at all — nothing broken to fix, but worth deciding whether that's intended.