chore: add types-lhci and related files#210
Conversation
``` chore: add types-lhci and related files ```
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Summary of ChangesHello @stephansama, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughA new TypeScript package for LHCI type definitions is being created with zod-based schemas for configuration validation, including build scripts, documentation, examples, and necessary configuration files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new package, types-lhci, to provide Zod schemas for Lighthouse CI configuration. The changes are well-structured, including the new package source, build scripts, and documentation updates. I've provided some feedback to improve the correctness and usability of the Zod schemas, particularly around optional properties and providing more specific types. Additionally, this PR includes a significant cleanup of pnpm-lock.yaml which should ideally be mentioned in the description to provide more context.
| | [typed-events](core/typed-events/README.md) |  |  | Typed events store using standard schema | | ||
| | [typed-nocodb-api](core/typed-nocodb-api/README.md) |  |  | Typed API client for NocoDB using Zod | | ||
| | [typed-templates](core/typed-templates/README.md) |  |  | Use standard schema to validate and use handlebar template directories | | ||
| | [types-lhci](core/types-lhci/README.md) |  |  | types for lhci configuration | |
There was a problem hiding this comment.
For consistency with the other package descriptions in this table, consider capitalizing the first letter of the description.
| | [types-lhci](core/types-lhci/README.md) |  |  | types for lhci configuration | | |
| | [types-lhci](core/types-lhci/README.md) |  |  | Types for lhci configuration | |
| ```javascript | ||
| import { lhciSchema } from "@stephansama/types-lhci"; | ||
|
|
||
| /** @type {import('../dist/index.d.cts').LhciSchema} */ |
There was a problem hiding this comment.
The JSDoc import path ../dist/index.d.cts is suitable for local development but can be confusing for external consumers of your package. It's better to use the package name in the example to show how it would be used in a real-world scenario.
| /** @type {import('../dist/index.d.cts').LhciSchema} */ | |
| /** @type {import('@stephansama/types-lhci').LhciSchema} */ |
| "peerDependencies": { | ||
| "zod": "catalog:schema" | ||
| }, | ||
| "packageManager": "pnpm@10.11.0", |
There was a problem hiding this comment.
Pinning the packageManager to a specific patch version can lead to issues for developers using a different patch version of pnpm. It's generally recommended to either define this at the root of the monorepo or use a less strict version range to ensure broader compatibility within your team.
| "packageManager": "pnpm@10.11.0", | |
| "packageManager": "pnpm@>=10.11.0", |
| timeout: z.number().default(30000), | ||
| }), | ||
| puppeteerScript: z.string(), | ||
| settings: z.object({}), |
There was a problem hiding this comment.
The schema for settings is currently an empty object (z.object({})), which provides no type safety for the Lighthouse settings that can be passed here. While the full schema is complex, using z.record(z.string(), z.any()) would be more descriptive and accurate, indicating it's a key-value object. For even better type safety, you could consider defining a more detailed schema for common Lighthouse settings.
| settings: z.object({}), | |
| settings: z.record(z.string(), z.any()), |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
More templates
@stephansama/ai-commit-msg
@stephansama/alfred-kaomoji
@stephansama/astro-iconify-svgmap
@stephansama/auto-readme
@stephansama/catppuccin-opml
@stephansama/catppuccin-rss
@stephansama/catppuccin-typedoc
@stephansama/catppuccin-xsl
create-stephansama-example
@stephansama/find-makefile-targets
@stephansama/github-env
@stephansama/multipublish
@stephansama/prettier-plugin-handlebars
@stephansama/remark-asciinema
@stephansama/svelte-social-share-links
@stephansama/typed-events
@stephansama/typed-nocodb-api
@stephansama/typed-templates
@stephansama/types-lhci
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@core/types-lhci/build.ts`:
- Around line 14-18: The current build converts the Zod schema to JSON then
calls JSON.stringify(jsonSchema) producing a single-line blob; update the
serialization to pretty-print by using JSON.stringify with a spacing argument
(e.g., JSON.stringify(jsonSchema, null, 2)) wherever jsonString is created
(refer to symbols schemas, jsonSchema, jsonString and the z.toJSONSchema call)
so the emitted schema.json is indented and human-readable.
- Around line 14-16: Add a null/undefined guard after the dynamic import of
"./dist/index.js" to ensure schemas.lhciSchema exists before calling
z.toJSONSchema: after awaiting import("./dist/index.js") check if
schemas.lhciSchema is present and throw a clear Error("lhciSchema not found in
build output") (or similar) if not, then call
z.toJSONSchema(schemas.lhciSchema); this prevents confusing failures when the
build output is malformed.
In `@core/types-lhci/package.json`:
- Line 36: The dev script in package.json currently references a non-existent
path "./src/build.ts"; update the "dev" NPM script entry (the "dev" property) to
use the correct path "./build.ts" (same as the "build" script) so running npm
run dev invokes tsx --watch ./build.ts instead of the missing ./src/build.ts.
- Around line 21-27: The package.json exports map is missing a "types" condition
which prevents TypeScript (with moduleResolution "node16"/"bundler") from
resolving declarations; update the "exports" "." entry to include a "types"
condition pointing to the built d.ts file (e.g. add "types": "./dist/index.d.ts"
alongside "import" and "require") so consumers use the correct types when
"exports" is present, and keep the top-level "types" field as a fallback.
In `@core/types-lhci/README.md`:
- Line 10: The "Table of contents" heading uses ##### (h5) which skips from the
document h1; change that heading to the next appropriate level (e.g., ## or ###)
so it increments correctly from the existing h1 in core/types-lhci/README.md —
update the "Table of contents" heading markup to use a proper h2/h3 instead of
h5 to satisfy MD001 and match other repo READMEs.
- Around line 27-42: The README's JSDoc type annotation imports the internal
dist path; replace the internal import('../dist/index.d.cts').LhciSchema with
the published package name import (e.g.
import('@stephansama/types-lhci').LhciSchema) so consumers see the public API;
update the example that declares config (and any references to lhciSchema or
LhciSchema) to use the package-level type import instead of the dist path.
In `@core/types-lhci/src/index.ts`:
- Line 38: The schema field "method" currently uses z.enum(["node",
"psi"]).or(z.string()).default("node") which makes the enum redundant; either
restrict to known values by removing .or(z.string()) and use z.enum(["node",
"psi"]).default("node"), or allow any string by replacing the construct with
z.string().default("node"); update the "method" declaration accordingly to
reflect the intended validation behavior (choose one of the two replacements and
remove the redundant part).
- Around line 88-95: The intersected z.object used with .and() currently makes
githubAppToken and githubToken required for every upload target; update that
z.object so GitHub-specific credentials are optional by changing githubAppToken
and githubToken to optional (e.g., z.string().optional()), keeping githubApiHost
default and githubStatusContextSuffix optional, so GitHub fields no longer cause
validation failures when not provided.
- Line 50: The schema's settings field currently uses z.object({}) which strips
unknown keys and will reject real Lighthouse settings; update the "settings"
schema (in the Zod object where the symbol/settings property is defined) to
accept arbitrary key/value pairs—either replace z.object({}) with
z.record(z.string(), z.unknown()) or call .passthrough() on the object (e.g.,
z.object({}).passthrough()) so incoming Lighthouse settings are preserved.
- Around line 10-30: The lhciAssertSchema currently requires every field and
will reject valid LHCI configs; update lhciAssertSchema so assertions,
budgetsFile, includePassedAssertions, and preset are optional (use .optional()),
and ensure ci.collect-related schemas (e.g., the collect schema for url,
numberOfRuns, chromePath, headful, additive, puppeteerScript,
startServerCommand, staticDirFileDiscoveryDepth) mark their fields optional with
sensible defaults where applicable; remove the redundant pattern
z.enum(["node","psi"]).or(z.string()) by using a single z.string() or a stricter
enum, and replace any z.object({}) empty strict objects with
z.object({}).passthrough() or z.record(...) to allow arbitrary properties; also
make githubAppToken and githubToken optional in the upload/github integration
combinator so upload targets without GitHub tokens validate.
| const schemas = await import("./dist/index.js"); | ||
|
|
||
| const jsonSchema = z.toJSONSchema(schemas.lhciSchema); | ||
|
|
||
| const jsonString = JSON.stringify(jsonSchema); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider pretty-printing the JSON schema output.
The generated schema.json will be a single-line blob, making it hard to inspect or diff. Consider adding indentation for readability.
Proposed fix
-const jsonString = JSON.stringify(jsonSchema);
+const jsonString = JSON.stringify(jsonSchema, null, 2);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const schemas = await import("./dist/index.js"); | |
| const jsonSchema = z.toJSONSchema(schemas.lhciSchema); | |
| const jsonString = JSON.stringify(jsonSchema); | |
| const schemas = await import("./dist/index.js"); | |
| const jsonSchema = z.toJSONSchema(schemas.lhciSchema); | |
| const jsonString = JSON.stringify(jsonSchema, null, 2); |
🤖 Prompt for AI Agents
In `@core/types-lhci/build.ts` around lines 14 - 18, The current build converts
the Zod schema to JSON then calls JSON.stringify(jsonSchema) producing a
single-line blob; update the serialization to pretty-print by using
JSON.stringify with a spacing argument (e.g., JSON.stringify(jsonSchema, null,
2)) wherever jsonString is created (refer to symbols schemas, jsonSchema,
jsonString and the z.toJSONSchema call) so the emitted schema.json is indented
and human-readable.
| const schemas = await import("./dist/index.js"); | ||
|
|
||
| const jsonSchema = z.toJSONSchema(schemas.lhciSchema); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check zod version in package.json files
fd package.json --max-depth 3 --exec cat {} \; 2>/dev/null | grep -A2 -B2 '"zod"'
# Also check yarn.lock or package-lock for zod version details
if [ -f "yarn.lock" ]; then
echo "=== Yarn lock (zod entries) ==="
grep -A5 'name: "zod"' yarn.lock | head -20
fi
if [ -f "package-lock.json" ]; then
echo "=== Package lock (zod entries) ==="
jq '.dependencies.zod // .packages["node_modules/zod"] | select(. != null)' package-lock.json 2>/dev/null || grep -A5 '"zod"' package-lock.json | head -20
fiRepository: stephansama/packages
Length of output: 1180
🏁 Script executed:
#!/bin/bash
# Check if z.toJSONSchema is referenced elsewhere
rg -n 'toJSONSchema' --type=ts --type=tsx --type=js
# Also check the build.ts file content for full context
echo "=== Full build.ts file ==="
fd build.ts -x cat {} \;Repository: stephansama/packages
Length of output: 834
Add null check for schemas.lhciSchema before passing to z.toJSONSchema.
The dynamic import of ./dist/index.js is fragile—if the build output is malformed or the import throws, the error will be confusing. Additionally, schemas.lhciSchema is used without checking if it exists. Add a guard:
if (!schemas.lhciSchema) {
throw new Error("lhciSchema not found in build output");
}
const jsonSchema = z.toJSONSchema(schemas.lhciSchema);The z.toJSONSchema API is available in the project's required zod version (≥4 per peerDependencies), so no action needed there.
🤖 Prompt for AI Agents
In `@core/types-lhci/build.ts` around lines 14 - 16, Add a null/undefined guard
after the dynamic import of "./dist/index.js" to ensure schemas.lhciSchema
exists before calling z.toJSONSchema: after awaiting import("./dist/index.js")
check if schemas.lhciSchema is present and throw a clear Error("lhciSchema not
found in build output") (or similar) if not, then call
z.toJSONSchema(schemas.lhciSchema); this prevents confusing failures when the
build output is malformed.
| "exports": { | ||
| ".": { | ||
| "import": "./dist/index.js", | ||
| "require": "./dist/index.cjs" | ||
| }, | ||
| "./package.json": "./package.json" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's see the full package.json for context
echo "=== Full core/types-lhci/package.json ==="
cat core/types-lhci/package.jsonRepository: stephansama/packages
Length of output: 1273
🏁 Script executed:
#!/bin/bash
# Check how other packages in this repo handle exports + types
echo "=== Exports + types patterns in core packages ==="
rg -n '"types"' --glob 'core/*/package.json' -A3 -B3Repository: stephansama/packages
Length of output: 7636
🏁 Script executed:
#!/bin/bash
# Check what declaration files exist in the dist directory
echo "=== Looking for dist declaration files ==="
find core/types-lhci/dist -name "*.d.*" 2>/dev/null || echo "dist directory not found or no .d.* files"
git ls-files core/types-lhci/dist 2>/dev/null | grep -E '\.(d\.ts|d\.cts)$' || echo "No declaration files tracked in git"Repository: stephansama/packages
Length of output: 184
Add missing types condition to exports map.
The exports field doesn't include a types condition. TypeScript consumers using moduleResolution: "node16" or "bundler" rely on the types condition in the exports map to resolve type declarations. The top-level "types" field (line 30) is a fallback but isn't consulted when exports are present.
Proposed fix
"exports": {
".": {
+ "types": "./dist/index.d.cts",
"import": "./dist/index.js",
"require": "./dist/index.cjs"
},
"./package.json": "./package.json"
},This pattern is already used in other packages in the repo like svelte-social-share-links, auto-readme, and ai-commit-msg.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "exports": { | |
| ".": { | |
| "import": "./dist/index.js", | |
| "require": "./dist/index.cjs" | |
| }, | |
| "./package.json": "./package.json" | |
| }, | |
| "exports": { | |
| ".": { | |
| "types": "./dist/index.d.cts", | |
| "import": "./dist/index.js", | |
| "require": "./dist/index.cjs" | |
| }, | |
| "./package.json": "./package.json" | |
| }, |
🤖 Prompt for AI Agents
In `@core/types-lhci/package.json` around lines 21 - 27, The package.json exports
map is missing a "types" condition which prevents TypeScript (with
moduleResolution "node16"/"bundler") from resolving declarations; update the
"exports" "." entry to include a "types" condition pointing to the built d.ts
file (e.g. add "types": "./dist/index.d.ts" alongside "import" and "require") so
consumers use the correct types when "exports" is present, and keep the
top-level "types" field as a fallback.
| ], | ||
| "scripts": { | ||
| "build": "tsx ./build.ts", | ||
| "dev": "tsx --watch ./src/build.ts", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the actual location of build.ts
fd -t f 'build.ts' core/types-lhci/Repository: stephansama/packages
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Check the package.json content around the dev and build scripts
cat core/types-lhci/package.jsonRepository: stephansama/packages
Length of output: 1231
Bug: dev script points to non-existent file.
The dev script references ./src/build.ts, but build.ts is located at the package root. The build script correctly uses ./build.ts, so the dev script should too.
🐛 Proposed fix
- "dev": "tsx --watch ./src/build.ts",
+ "dev": "tsx --watch ./build.ts",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dev": "tsx --watch ./src/build.ts", | |
| "dev": "tsx --watch ./build.ts", |
🤖 Prompt for AI Agents
In `@core/types-lhci/package.json` at line 36, The dev script in package.json
currently references a non-existent path "./src/build.ts"; update the "dev" NPM
script entry (the "dev" property) to use the correct path "./build.ts" (same as
the "build" script) so running npm run dev invokes tsx --watch ./build.ts
instead of the missing ./src/build.ts.
|
|
||
| types for lhci configuration | ||
|
|
||
| ##### Table of contents |
There was a problem hiding this comment.
Heading level skips from h1 to h5.
The ##### heading jumps from h1 directly to h5, violating heading-level increment rules (MD001). This matches the pattern used in other READMEs in this repo (e.g., core/auto-readme/README.md line 10), so it may be intentional for visual styling — but it does break accessibility and markdown linting.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 10-10: Heading levels should only increment by one level at a time
Expected: h2; Actual: h5
(MD001, heading-increment)
🤖 Prompt for AI Agents
In `@core/types-lhci/README.md` at line 10, The "Table of contents" heading uses
##### (h5) which skips from the document h1; change that heading to the next
appropriate level (e.g., ## or ###) so it increments correctly from the existing
h1 in core/types-lhci/README.md — update the "Table of contents" heading markup
to use a proper h2/h3 instead of h5 to satisfy MD001 and match other repo
READMEs.
| ```javascript | ||
| import { lhciSchema } from "@stephansama/types-lhci"; | ||
|
|
||
| /** @type {import('../dist/index.d.cts').LhciSchema} */ | ||
| const config = { | ||
| ci: { | ||
| upload: { | ||
| githubAppToken: process.env.GITHUB_APP_TOKEN || "fallback", | ||
| githubToken: process.env.GITHUB_TOKEN || "fallback", | ||
| serverBaseUrl: "https://lhci.example.com", | ||
| target: "lhci", | ||
| token: "project-token", | ||
| }, | ||
| }, | ||
| }; | ||
| ``` |
There was a problem hiding this comment.
Usage example shows internal dist path for JSDoc type annotation.
Line 30 uses import('../dist/index.d.cts').LhciSchema which is an internal path. Published consumers should reference the package name instead. This is user-facing documentation, so it should demonstrate the expected consumer experience.
Suggested fix
-/** `@type` {import('../dist/index.d.cts').LhciSchema} */
+/** `@type` {import('@stephansama/types-lhci').LhciSchema} */🤖 Prompt for AI Agents
In `@core/types-lhci/README.md` around lines 27 - 42, The README's JSDoc type
annotation imports the internal dist path; replace the internal
import('../dist/index.d.cts').LhciSchema with the published package name import
(e.g. import('@stephansama/types-lhci').LhciSchema) so consumers see the public
API; update the example that declares config (and any references to lhciSchema
or LhciSchema) to use the package-level type import instead of the dist path.
| chromePath: z.string(), | ||
| headful: z.boolean(), | ||
| isSinglePageApplication: z.boolean(), | ||
| method: z.enum(["node", "psi"]).or(z.string()).default("node"), |
There was a problem hiding this comment.
z.enum(["node", "psi"]).or(z.string()) — the enum is redundant.
Since .or(z.string()) accepts any string, the preceding z.enum(...) is entirely subsumed and serves no validation purpose. If the intent is to document known values while still allowing arbitrary strings, consider just using z.string().default("node"). If the intent is to restrict to known values, remove .or(z.string()).
🤖 Prompt for AI Agents
In `@core/types-lhci/src/index.ts` at line 38, The schema field "method" currently
uses z.enum(["node", "psi"]).or(z.string()).default("node") which makes the enum
redundant; either restrict to known values by removing .or(z.string()) and use
z.enum(["node", "psi"]).default("node"), or allow any string by replacing the
construct with z.string().default("node"); update the "method" declaration
accordingly to reflect the intended validation behavior (choose one of the two
replacements and remove the redundant part).
| timeout: z.number().default(30000), | ||
| }), | ||
| puppeteerScript: z.string(), | ||
| settings: z.object({}), |
There was a problem hiding this comment.
Empty z.object({}) will reject any Lighthouse settings.
z.object({}) in Zod strips unknown keys by default, so any actual Lighthouse settings passed here will be silently dropped. Use z.record(z.string(), z.unknown()) or z.object({}).passthrough() to allow arbitrary settings.
Proposed fix
- settings: z.object({}),
+ settings: z.record(z.string(), z.unknown()).optional(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| settings: z.object({}), | |
| settings: z.record(z.string(), z.unknown()).optional(), |
🤖 Prompt for AI Agents
In `@core/types-lhci/src/index.ts` at line 50, The schema's settings field
currently uses z.object({}) which strips unknown keys and will reject real
Lighthouse settings; update the "settings" schema (in the Zod object where the
symbol/settings property is defined) to accept arbitrary key/value pairs—either
replace z.object({}) with z.record(z.string(), z.unknown()) or call
.passthrough() on the object (e.g., z.object({}).passthrough()) so incoming
Lighthouse settings are preserved.
1 participant
Checklist
mainhave been mergedmain