chore(deps-dev): bump the eslint group across 1 directory with 5 updates#2431
chore(deps-dev): bump the eslint group across 1 directory with 5 updates#2431dependabot[bot] wants to merge 11 commits intomainfrom
Conversation
dependabot
bot
added
area: dependencies
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
github-actions
bot
added
area: showcase
dependabot
bot
force-pushed
the
dependabot/npm_and_yarn/eslint-a4f23f293e
branch
from
05b93c7 to
fcbbf4d
Compare
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (43.75%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## main #2431 +/- ##
=========================================
+ Coverage 0 42.06% +42.06%
=========================================
Files 0 567 +567
Lines 0 20215 +20215
Branches 0 6122 +6122
=========================================
+ Hits 0 8504 +8504
- Misses 0 11623 +11623
- Partials 0 88 +88
... and 553 files with indirect coverage changes
🚀 New features to boost your workflow:
|
dependabot
bot
force-pushed
the
dependabot/npm_and_yarn/eslint-a4f23f293e
branch
from
fcbbf4d to
376ebbd
Compare
dependabot
bot
force-pushed
the
dependabot/npm_and_yarn/eslint-a4f23f293e
branch
from
376ebbd to
aec5566
Compare
dependabot
bot
force-pushed
the
dependabot/npm_and_yarn/eslint-a4f23f293e
branch
from
aec5566 to
ba25c74
Compare
|
Pushed follow-up fixes to get both Changes
Verificationnpm ci
npm run lint
npm run check-types
npm test
npm run build
Re-requested review from You can see my work on the |
![charliecreates[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
cli/src/commands/auth.tsnow leavesexitCodeuninitialized, which can produce incorrect exit behavior for unknown subcommands.create-tambo-app’s Node engine range (>=22) is incompatible with ESLint v10’s published engine requirement (^22.13.0 || >=24), risking install/lint failures for Node22.0–22.12.- React version workaround logic is duplicated across multiple ESLint configs and relies on
process.cwd(), making it easy to drift and potentially fragile under different invocation patterns.
Additional notes (1)
- Compatibility |
create-tambo-app/package.json:42-50
create-tambo-appdeclares"node": ">=22", but ESLint v10 (and its@eslint/*packages) require Node^20.19.0 || ^22.13.0 || >=24(visible in the lockfile). Allowing Node22.0.0–22.12.xmeans installs/lint may fail due to engine checks or runtime incompatibilities. For a generator package, this is a real footgun.
Summary of changes
ESLint v10 migration + follow-up fixes
- Upgraded dev tooling packages to ESLint v10 across the repo (e.g.
eslint,@eslint/js,typescript-eslint,eslint-plugin-jsdoc,eslint-plugin-turbo) and regeneratedpackage-lock.jsonaccordingly. - Updated
showcaseto useeslint: "^10". - Added explicit
Errorcauses in multiple CLI commands (new Error(message, { cause })) to preserve underlying errors. - Hardened component file installation by allowing
fileContentto beundefinedand throwing a clearer error when content can’t be resolved. - Adjusted React marker cluster badge sizing logic and documented thresholds.
- Updated shared ESLint configs (
packages/eslint-config/*+react-sdk/eslint.config.mjs) to avoidreactversion auto-detection on ESLint 10 by resolving React fromprocess.cwd()viacreateRequire. - Removed
/* eslint-env node */from a Node script inshowcase/scripts/format-code-snippets.mjs(relying on ESMnode:imports instead).
| const requireFromCwd = createRequire(`${process.cwd()}/package.json`); | ||
| const eslintMajorVersion = Number.parseInt( | ||
| requireFromCwd("eslint/package.json").version.split(".")[0] ?? "0", | ||
| 10, | ||
| ); | ||
| const installedReactVersion = (() => { | ||
| try { | ||
| return requireFromCwd("react/package.json").version; | ||
| } catch { | ||
| return "999.999.999"; | ||
| } | ||
| })(); | ||
| const reactConfigVersion = | ||
| eslintMajorVersion >= 10 ? installedReactVersion : "detect"; | ||
|
|
There was a problem hiding this comment.
The React version resolution logic is duplicated across multiple configs (next.mjs, react-internal.mjs, react-sdk/eslint.config.mjs). This is brittle: future changes (e.g., switching the sentinel version, changing how process.cwd() behaves under different runners, or updating the ESLint major check) will require editing multiple files and can easily drift.
Also: using process.cwd() assumes the linter is invoked with each package as CWD. That may be true for turbo lint, but it’s not universally true (e.g., running eslint from repo root with --config packages/eslint-config/next.mjs).
Suggestion
Deduplicate the logic by extracting a small helper module inside packages/eslint-config and importing it from each config.
Example:
// packages/eslint-config/react-version.mjs
import { createRequire } from "node:module";
import process from "node:process";
export function getReactConfigVersion() {
const requireFromCwd = createRequire(`${process.cwd()}/package.json`);
const eslintMajor = Number.parseInt(
requireFromCwd("eslint/package.json").version.split(".")[0] ?? "0",
10,
);
if (eslintMajor < 10) return "detect";
try {
return requireFromCwd("react/package.json").version;
} catch {
return "999.999.999";
}
}Then in configs:
import { getReactConfigVersion } from "./react-version.mjs";
// ...
settings: { react: { version: getReactConfigVersion() } },Optionally, consider using import.meta.url-relative resolution (or createRequire(import.meta.url)) and walking upward to find the closest package root instead of relying on CWD.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| } catch (error) { | ||
| throw new Error(`Failed to install dependencies: ${error}`); | ||
| throw new Error(`Failed to install dependencies: ${error}`, { | ||
| cause: error, | ||
| }); |
There was a problem hiding this comment.
Throwing new Error(..., { cause }) is a good direction, but here the thrown error message includes ${error} which will stringify as [object Object] for non-Error throws. In other places you guard error instanceof Error and use error.message.
If this catch might receive non-Error values (e.g. libraries that throw strings/objects), the resulting message can become low-value while still being user-visible.
Suggestion
Prefer normalizing the message similarly to your other catch blocks:
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
throw new Error(`Failed to install dependencies: ${message}`, { cause: error });
}This keeps the user-facing message informative while preserving the original error via cause. Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this suggestion.
| if (fileContent === undefined) { | ||
| throw new Error( | ||
| `Failed to resolve content for '${file.name}' (sourcePath='${sourcePath}')`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
This new guard is helpful, but the thrown message only mentions sourcePath even though resolution now tries multiple sources (contentPath derived from registry base, and literal file.content). When this fails in the wild, you’ll want all the attempted paths in the error to debug quickly.
Also, it might be possible for file.content to be null (depending on upstream schema); in that case the current check won’t throw, and updateImportPaths/writeFileSync will receive a non-string at runtime.
Suggestion
Include the computed contentPath (and whether you fell back to literal content) in the error message, and harden the guard to require a string:
if (typeof fileContent !== "string") {
throw new Error(
`Failed to resolve content for '${file.name}' (sourcePath='${sourcePath}', contentPath='${contentPath}')`,
);
}If contentPath is only defined in the else branch, consider declaring it outside as let contentPath: string | undefined; so it can be referenced in the error message.
Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this suggestion.
|
@CharlieHelps lint is still failing. |
|
Investigating why lint is still failing on PR #2431 now, and I may review the recent CI logs and eslint config changes to pinpoint what’s breaking.
|
Expand this to see my work.
|
github-actions
bot
added
the
area: backend
|
Tracked the failing Changes
Verification# ESLint: 17/17 tasks successful
npm run lint
# TypeScript: 19/19 tasks successful
npm run check-types
# Tests: 14/14 tasks successful
npm testSelf-review notes:
Re-requested review from |
There was a problem hiding this comment.
The biggest correctness risk is in cli/src/commands/auth.ts where exitCode is no longer initialized, making it easier to accidentally report success for invalid/unknown subcommands. Several CLI rethrows interpolate ${error} and can produce unhelpful [object Object] messages despite using { cause }. The React version workaround logic is duplicated across multiple ESLint config files and relies on process.cwd(), which is fragile outside the expected invocation pattern. create-tambo-app’s engines.node range is too broad for ESLint v10’s published engine requirement and should be tightened to avoid install/runtime failures on Node 22.0–22.12.
Additional notes (2)
- Maintainability |
cli/src/commands/auth.ts:531-534
exitCodeis no longer initialized. This is a behavioral change: if an unknownsubcommandfalls through (or a future refactor adds a branch that doesn’t assignexitCode), you risk callingprocess.exitCode = exitCode(or equivalent) withundefinedor leaving it unchanged, which can incorrectly report success.
Given this is a CLI entrypoint, the safe default is to assume failure for unknown/invalid usage and/or explicitly handle the else case by showing help + non-zero exit.
This is not a theoretical style concern—this variable previously had a safe default (0) and now does not, making it easier to introduce a silent success path.
- Compatibility |
create-tambo-app/package.json:42-50
create-tambo-appdeclaresengines.node: ">=22", but ESLint v10’s engine requirement (visible in the lockfile) is^20.19.0 || ^22.13.0 || >=24. That means Node22.0.0–22.12.xsatisfies your package engine but can fail installation or runtime due to ESLint’s own engine constraints.
For a generator package, this is a real UX footgun because users often run whatever Node 22 they have installed.
Summary of changes
Overview
This diff primarily completes an ESLint v10 migration and then adjusts application code to satisfy new/stricter lint rules.
Key changes
- CLI error handling: multiple
catchblocks now rethrow withnew Error(message, { cause })to preserve underlying exceptions. - CLI file installation: component file copying now treats
fileContentas potentially missing and throws a clearer error when content can’t be resolved. - Auth command flow: changed
exitCodefrom initialized (0) to declared-only. - Backend lint fixes:
- Removed a useless assignment in
packages/backend/src/services/llm/agent-client.ts. - Preserved caught error as
causein thread-name parsing. - Improved parse error messaging and preserved
causein component streaming.
- Removed a useless assignment in
- ESLint config workaround for ESLint 10 + react plugin:
settings.react.versionis now computed viacreateRequire(process.cwd())(fallback `
| } catch (error) { | ||
| throw new Error(`Failed to merge configs: ${error}`); | ||
| throw new Error(`Failed to merge configs: ${error}`, { | ||
| cause: error, | ||
| }); |
There was a problem hiding this comment.
Same issue as other CLI catch blocks: interpolating ${error} can yield [object Object] and loses signal for users. Since you’re already using { cause }, keep the user message clean and deterministic.
Suggestion
Normalize the message similarly:
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
throw new Error(`Failed to merge configs: ${message}`, { cause: error });
}Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this suggestion.
|
@CharlieHelps still failing. Run |
Expand this to see my work.
|
|
Fixed the remaining Pushed in Changes
Verification# ESLint: pass (0 errors; 16 warnings remain)
npm run lint -- --filter=@tambo-ai/react
# TypeScript: pass
npm run check-types -- --filter=@tambo-ai/react
# Jest: 59 test suites passed; 1002 tests passed
npm test -- --filter=@tambo-ai/reactreviewChanges skipped:
|
There was a problem hiding this comment.
cli/src/commands/auth.tsnow has an uninitializedexitCode, which is a correctness risk for unknown/future subcommand branches.- Several CLI rethrows still interpolate
${error}while also using{ cause }, producing low-signal[object Object]messages for non-Errorthrows. - The ESLint v10 React-version workaround is duplicated across configs and relies on
process.cwd(), which is brittle under alternate invocation patterns. create-tambo-app’sengines.node(>=22) is incompatible with ESLint v10’s Node requirement (^22.13.0 || >=24), risking install/runtime failures on Node22.0–22.12.
Additional notes (1)
- Compatibility |
create-tambo-app/package.json:42-50
Compatibility issue:create-tambo-appdeclaresengines.node: ">=22", but ESLint v10 (and@eslint/*packages) require Node^20.19.0 || ^22.13.0 || >=24(visible in the lockfile additions). This means Node22.0.0–22.12.xsatisfies your package engine but can fail installs or runtime when ESLint is pulled in.
For a generator package, that’s a real footgun: users commonly have “some Node 22” installed.
Summary of changes
What changed
ESLint v10 migration (tooling)
- Updated dev tooling versions in multiple workspaces:
create-tambo-app/package.json:eslint→^10.0.0,@eslint/js→^10.0.1,typescript-eslint→^8.56.0.packages/eslint-config/package.json: same bumps pluseslint-plugin-turbo→^2.8.10.react-sdk/package.json:eslint→^10.0.0,eslint-plugin-jsdoc→^62.6.1,typescript-eslint→^8.56.0.showcase/package.json:eslint→^10.
- Regenerated
package-lock.jsonreflecting the above upgrades and new ESLint v10 transitive packages (@eslint/*,espree@11,minimatch@10, etc.).
ESLint config workarounds for React detection under ESLint 10
packages/eslint-config/next.mjs,packages/eslint-config/react-internal.mjs, andreact-sdk/eslint.config.mjsnow:- Import
createRequire+process(node:builtins). - Read the installed ESLint major version.
- Use explicit
settings.react.versionbased on the installed React version for ESLint>=10; keep"detect"for ESLint<10.
- Import
Lint-rule driven code changes
- CLI: multiple
catchblocks now rethrow withnew Error(message, { cause: error })to satisfypreserve-caught-error. - CLI add-component:
fileContentcan now beundefinedand throws when content can’t be resolved. - CLI auth:
exitCodeis no longer initialized to0. - Backend:
- Removed a useless assignment in
AgentClient. thread-name.service.tsnow throws with{ cause }(instead ofconsole.error+ generic throw).component-streaming.tsnow includes normalized non-Errorthrowables and{ cause }.
- Removed a useless assignment in
UI registry
- Simplified marker-cluster sizing logic and documented cluster thresholds.
Misc
- Removed
/* eslint-env node */fromshowcase/scripts/format-code-snippets.mjs(flat-config / ESLint 10 compatibility).
Bumps the eslint group with 5 updates in the / directory:
9.39.210.0.19.39.210.0.062.5.462.5.58.54.08.55.02.8.32.8.9Updates
@eslint/jsfrom 9.39.2 to 10.0.1Release notes
Sourced from
@eslint/js's releases.... (truncated)
Commits
Updates
eslintfrom 9.39.2 to 10.0.0Release notes
Sourced from eslint's releases.
... (truncated)
Commits
4e6c4ac10.0.0ddd8a22Build: changelog update for 10.0.0bff9091feat: handleArray.fromAsyncinarray-callback-return(#20457)1ece282chore: ignore/docs/v9.xin link checker (#20452)034e139ci: add type integration test for@html-eslint/eslint-plugin(#20345)f3fbc2fchore: set@eslint/jsversion to 10.0.0 to skip releasing it (#20466)e978ddadocs: Update README4cecf83docs: Update READMEc79f0abdocs: Update READMEafc0681chore: remove scopeManager.addGlobals patch for typescript-eslint parser (#20...Updates
eslint-plugin-jsdocfrom 62.5.4 to 62.5.5Release notes
Sourced from eslint-plugin-jsdoc's releases.
Commits
c7b132ffix(check-param-names): check arrow function properties in interfaces (TSPr...Updates
typescript-eslintfrom 8.54.0 to 8.55.0Release notes
Sourced from typescript-eslint's releases.
Changelog
Sourced from typescript-eslint's changelog.
Commits
fedfe86chore(release): publish 8.55.0b931f8cchore: use workspace refs for workspace deps (#12018)1f17a79chore: migrate to pnpm (#11248)Updates
eslint-plugin-turbofrom 2.8.3 to 2.8.9Release notes
Sourced from eslint-plugin-turbo's releases.
... (truncated)
Commits
4c5341frelease(turborepo): 2.8.9 (#11865)6e151bcrelease(turborepo): 2.8.9-canary.2 (#11859)0f740b6release(turborepo): 2.8.9-canary.1 (#11858)a1a3e74fix: Recover version numbering to 2.8.9-canary.0 (#11853)356b995release(turborepo): 2.8.8-canary.8 (#11852)f0f7394release(turborepo): 2.8.8-canary.7 (#11846)c19d080release(turborepo): 2.8.8-canary.6 (#11843)b7d8795release(turborepo): 2.8.8-canary.5 (#11842)45fbbd0release(turborepo): 2.8.8-canary.4 (#11839)959edberelease(turborepo): 2.8.8-canary.3 (#11834)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore <dependency name> major versionwill close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself)@dependabot ignore <dependency name> minor versionwill close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself)@dependabot ignore <dependency name>will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself)@dependabot unignore <dependency name>will remove all of the ignore conditions of the specified dependency@dependabot unignore <dependency name> <ignore condition>will remove the ignore condition of the specified dependency and ignore conditions