chore: add nocodb package and update cspell dictionary#200
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new monorepo package Changes
Sequence DiagramsequenceDiagram
participant Client as Client Code
participant Factory as createApi()
participant Fetch as API.fetch()
participant NocoDB as NocoDB Server
participant Validator as Zod Validator
Client->>Factory: createApi({ baseId, origin, schema, tableId, token? })
Factory-->>Client: API instance (with fetch)
Client->>Fetch: fetch({ action, query?, body?, token? })
Fetch->>Fetch: resolve token (param || stored)
Fetch->>Fetch: validate token present
Fetch->>Fetch: build URL & serialize query/body
Fetch->>NocoDB: HTTP request (xc-token header)
NocoDB-->>Fetch: HTTP response
Fetch->>Validator: validate response with responseSchema
Validator-->>Fetch: validated typed result
Fetch-->>Client: return typed response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
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 primarily focuses on expanding the project's utility by integrating a new NocoDB API client, which enhances interaction with NocoDB instances through a type-safe and schema-validated approach. Concurrently, it refines the development environment by updating ESLint plugins to their latest versions, ensuring better code quality and consistency across the codebase. 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
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
![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, typed-nocodb-api, which is a typed client for the NocoDB API. While it's a good start, the implementation has several critical issues, particularly with the READ and DELETE actions, which are currently unusable. There are also inconsistencies in data types for record IDs across different schemas, which will lead to runtime validation errors. The example usage also demonstrates some risky patterns, such as using non-null assertions on environment variables and not handling promises correctly. My review includes specific feedback and suggestions to address these issues and improve the robustness of the new package. The dependency updates for eslint-plugin-zod are correct.
I am having trouble creating individual review comments. Click here to see my feedback.
core/typed-nocodb-api/src/index.ts (44-49)
The DELETE action is incorrectly configured:
- It uses the
PATCHHTTP method, but it should beDELETE. - The
inputSchemaexpects a numericid, but NocoDB record IDs are strings. - The schema seems designed for a single record, but it targets a bulk endpoint. For bulk deletion, the API expects a body like
{"ids": ["id1", "id2"]}. - The
responseSchemais empty, but the API typically returns the count of deleted records.
DELETE: {
inputSchema: z.object({ ids: z.array(z.string()) }),
method: "delete",
responseSchema: z.object({ count: z.number() }),
url: `/api/v3/data/${baseId}/${tableId}/records`,
},
core/typed-nocodb-api/src/index.ts (70-74)
The READ action is not implemented correctly. The URL /api/v3/data/${baseId}/${tableId}/records/{recordId} contains a {recordId} placeholder that is never replaced by the fetch function. Furthermore, the props type for fetch does not allow passing a recordId. This makes the READ action unusable. You'll need to update the fetch function to accept a recordId for the READ action and construct the URL accordingly.
core/typed-nocodb-api/src/index.ts (66)
The response schema for the LIST action defines the record id as a number (z.number()). However, NocoDB record IDs are strings (e.g., recxxxxxxxxxxxxxx). This will cause a validation error when parsing the API response.
records: z.array(z.object({ fields: schema, id: z.string() })),
core/typed-nocodb-api/src/index.ts (72)
The responseSchema for the READ action defines the record id as a number (z.number()). However, NocoDB record IDs are strings. This will cause a validation error when parsing the API response.
responseSchema: z.object({ fields: schema, id: z.string() }),
core/typed-nocodb-api/src/index.ts (75-80)
The UPDATE action appears to be designed for a single record, but it's using the bulk update endpoint (/records). For a single record update, the URL should typically include the record ID, like /records/{recordId}. The current implementation might not work as expected for single record updates and could be confusing for users of this API.
core/typed-nocodb-api/src/index.ts (117-120)
The URL with query parameters is constructed by string concatenation (url + params on line 128), which is fragile. If current.url were to contain a query string, this would result in an invalid URL. It's safer to use the URLSearchParams API directly on the URL object.
After applying this suggestion, you should also remove let params = ""; on line 115 and change fetch(url + params, ...) to fetch(url, ...) on line 128.
if ("query" in props && "querySchema" in current && props.query) {
const parsed = current.querySchema.parse(props.query);
for (const [key, value] of Object.entries(parsed)) {
url.searchParams.append(key, String(value));
}
}
core/typed-nocodb-api/example/index.ts (10-21)
Using non-null assertions (!) on environment variables like process.env.NOCODB_BASE! and process.env.NOCODB_TABLE! is risky. If these environment variables are not set, the application will crash at runtime with a potentially unhelpful error. It's better to check for their existence and throw a more informative error message before using them. For example:
const baseId = process.env.NOCODB_BASE;
if (!baseId) {
throw new Error("NOCODB_BASE environment variable is not set.");
}
// ... then use baseIdcore/typed-nocodb-api/example/index.ts (23-27)
The api.fetch call is an async function, but its returned promise is not handled (e.g., with await or .then()). This is a "floating promise" and can lead to unhandled promise rejections and unexpected behavior. The function should be marked as async and the promise should be awaited.
export async function callApi() {
await api.fetch({
action: "LIST",
});
}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In `@core/typed-nocodb-api/example/index.js`:
- Line 3: The example imports directly from a build artifact
("../dist/index.cjs"), which is fragile; update the import to use the package
entry resolved via package.json exports by replacing the build-artifact import
with an import from the package name "@stephansama/typed-nocodb-api" so the
example works whether or not the project is pre-built and regardless of output
filename; adjust any relative import statements in
core/typed-nocodb-api/example/index.js to import from the package name instead.
- Around line 18-22: The callApi function currently calls the async api.fetch
without awaiting or returning its promise, causing unhandled rejections and no
result propagation; fix by making callApi async (add async to its declaration)
and either await the call (const res = await api.fetch(...); return res;) or
directly return the promise (return api.fetch(...);) so callers receive the
result and errors propagate correctly — locate the callApi function and the
api.fetch invocation to apply this change.
- Around line 5-16: The createApi call uses baseId and tableId from process.env
(process.env.NOCODB_BASE, process.env.NOCODB_TABLE) which can be undefined; add
a guard before calling createApi to validate those env vars and either throw a
clear error or supply safe defaults so createApi always receives strings; update
the initialization around createApi to check/normalize baseId and tableId (and
optionally NOCODB_TOKEN) and fail fast with a descriptive message if missing.
In `@core/typed-nocodb-api/package.json`:
- Line 26: The package.json currently contains an empty "dependencies" object
which is unnecessary; remove the "dependencies" key entirely from
core/typed-nocodb-api's package.json so the manifest only includes fields that
have values (leaving devDependencies, scripts, name, version, etc. intact),
ensuring the file remains valid JSON after deletion.
In `@core/typed-nocodb-api/README.md`:
- Line 10: The "Table of contents" heading in README.md uses a level-5 heading
(#####) which skips from the document H1 and violates markdownlint MD001; update
the heading to the correct incremental level (use ## Table of contents) so
headings increment by one and comply with MD001, ensuring the visible text
"Table of contents" remains unchanged.
- Line 8: Update the README header phrase "standard schema compatible nocodb
api" to use a hyphenated compound adjective and proper capitalization (e.g.,
"standard-schema-compatible NocoDB API") so the compound modifier before the
noun is grammatically correct; locate and replace the exact string "standard
schema compatible nocodb api" in the README.md content with the
hyphenated/capitalized version.
In `@core/typed-nocodb-api/src/index.ts`:
- Line 47: The schema definition uses z.object() with no arguments which will
throw at runtime; update the responseSchema occurrences to use an explicit empty
shape by replacing z.object() with z.object({}) in both places where
responseSchema is defined (the DELETE responseSchema and the UPDATE
responseSchema) so the Zod schema represents an empty object rather than calling
z.object with no params.
- Around line 36-42: The Zod schema for the record id is inconsistent across the
action definitions (CREATE, DELETE, LIST, READ, UPDATE) in index.ts; determine
the actual id type returned by NocoDB v3 (string or number) and update all
occurrences so they match: e.g., the CREATE.responseSchema records' id, UPDATE
inputSchema id, DELETE inputSchema id, LIST.responseSchema records' id, and
READ.responseSchema id should all use the same zod type (z.string() or
z.number()) and related array/object wrappers; ensure you change the referenced
symbols CREATE.responseSchema, UPDATE.inputSchema, DELETE.inputSchema,
LIST.responseSchema, and READ.responseSchema to the unified type.
- Around line 115-120: The current serialization uses
URLSearchParams(parsed).toString(), which coerces arrays into single
comma-separated values; update the logic in the block that sets params (using
parsed from current.querySchema.parse(props.query)) to build the query string by
iterating parsed's entries and appending values to a URLSearchParams instance:
for array values call append for each element (to produce repeated keys like
fields=a&fields=b), for scalar values append once, then set params = "?" +
usp.toString(); ensure you convert non-string primitives to strings when
appending.
- Around line 44-49: The DELETE action in the exported routes incorrectly sets
method: "patch" (likely copy-paste from UPDATE); update the DELETE route
definition in index.ts so the DELETE entry (with inputSchema: z.object({ id:
z.number() }), responseSchema, and url:
`/api/v3/data/${baseId}/${tableId}/records`) uses method: "delete" instead of
"patch" to match the NocoDB v3 delete endpoint.
- Around line 137-138: The code calls response.json() and immediately runs
current.responseSchema.parse(json) without checking HTTP status, which can turn
4xx/5xx responses into misleading Zod errors; update the logic around
response.json()/current.responseSchema.parse to first check response.ok (and
response.status), and if not ok read a safe error body or text and throw a clear
HTTPError (including response.status, response.statusText and any parsed error
body) before attempting to validate with current.responseSchema.parse so
successful parsing only runs for 2xx responses.
- Around line 128-135: The fetch call in index.ts is missing a Content-Type
header when sending a JSON body: update the request logic around the fetch that
uses current.method and body so that when body is present (for
POST/PATCH/DELETE) you add "Content-Type": "application/json" to the Headers and
ensure the body is JSON.stringify(body) (or already a string) before passing it
to fetch; leave GET requests unchanged. This targets the fetch invocation that
constructs new Headers({ "accept": "application/json", "xc-token": token }) and
the variables current.method and body.
In `@core/typed-nocodb-api/tsdown.config.ts`:
- Around line 3-11: The tsdown config omits skipNodeModulesBundle which causes
node_modules like zod to be bundled; update the defineConfig call in
tsdown.config.ts (the object passed to defineConfig) to add
skipNodeModulesBundle: true so external dependencies/peerDependencies (e.g.,
zod) are not inlined into the dist, matching other configs such as
core/ai-commit-msg/build.mjs and core/multipublish/build.mjs.
In `@eslint.config.ts`:
- Line 12: The imported variable name eslintPluginZodX is misleading because the
module is now "eslint-plugin-zod"; rename the import to eslintPluginZod and
update all usages (e.g., the plugin registration that currently references
eslintPluginZodX) to use eslintPluginZod instead so the identifier matches the
actual package name.
In `@README.md`:
- Line 62: The README entry for the workspace package [typed-nocodb-api] uses
the vague description "zod nocodb api"; update the package's source-of-truth
description in the package.json for `@stephansama/typed-nocodb-api` to a clearer
phrase such as "Typed API client for NocoDB using Zod" (or similar), then
regenerate or update the README table so the README row for typed-nocodb-api
reflects that new description; locate package.json in the core/typed-nocodb-api
module and the README table row that references typed-nocodb-api to ensure both
are consistent.
| @@ -0,0 +1,22 @@ | |||
| import * as z from "zod"; | |||
|
|
|||
| import { createApi } from "../dist/index.cjs"; | |||
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Importing from ../dist/index.cjs is fragile.
This requires the package to be pre-built and couples the example to a specific build output filename. Consider importing from the package name (@stephansama/typed-nocodb-api) instead, which would resolve via the exports field in package.json and work regardless of the build tool's output format.
🤖 Prompt for AI Agents
In `@core/typed-nocodb-api/example/index.js` at line 3, The example imports
directly from a build artifact ("../dist/index.cjs"), which is fragile; update
the import to use the package entry resolved via package.json exports by
replacing the build-artifact import with an import from the package name
"@stephansama/typed-nocodb-api" so the example works whether or not the project
is pre-built and regardless of output filename; adjust any relative import
statements in core/typed-nocodb-api/example/index.js to import from the package
name instead.
| const api = createApi({ | ||
| baseId: process.env.NOCODB_BASE, | ||
| origin: "https://nocodb.com", | ||
| schema: z.object({ | ||
| column1: z.string(), | ||
| column2: z.enum(["optionOne", "optionTwo", "optionThree"]), | ||
| column3: z.number(), | ||
| column4: z.boolean(), | ||
| }), | ||
| tableId: process.env.NOCODB_TABLE, | ||
| token: process.env.NOCODB_TOKEN, | ||
| }); |
There was a problem hiding this comment.
process.env values may be undefined where string is required.
createApi expects baseId: string and tableId: string, but process.env.NOCODB_BASE and process.env.NOCODB_TABLE are string | undefined. This will cause a type error in strict TS (though this is a .js file, the runtime behavior with undefined could produce malformed API URLs like /api/v3/data/undefined/undefined/records).
Consider adding guards or defaults:
🛡️ Proposed fix
+const baseId = process.env.NOCODB_BASE;
+const tableId = process.env.NOCODB_TABLE;
+const token = process.env.NOCODB_TOKEN;
+
+if (!baseId || !tableId || !token) {
+ throw new Error("NOCODB_BASE, NOCODB_TABLE, and NOCODB_TOKEN environment variables are required");
+}
+
const api = createApi({
- baseId: process.env.NOCODB_BASE,
+ baseId,
origin: "https://nocodb.com",
schema: z.object({
column1: z.string(),
column2: z.enum(["optionOne", "optionTwo", "optionThree"]),
column3: z.number(),
column4: z.boolean(),
}),
- tableId: process.env.NOCODB_TABLE,
- token: process.env.NOCODB_TOKEN,
+ tableId,
+ token,
});🤖 Prompt for AI Agents
In `@core/typed-nocodb-api/example/index.js` around lines 5 - 16, The createApi
call uses baseId and tableId from process.env (process.env.NOCODB_BASE,
process.env.NOCODB_TABLE) which can be undefined; add a guard before calling
createApi to validate those env vars and either throw a clear error or supply
safe defaults so createApi always receives strings; update the initialization
around createApi to check/normalize baseId and tableId (and optionally
NOCODB_TOKEN) and fail fast with a descriptive message if missing.
| export function callApi() { | ||
| api.fetch({ | ||
| action: "LIST", | ||
| }); | ||
| } |
There was a problem hiding this comment.
api.fetch is async but neither awaited nor returned.
The promise from api.fetch is fire-and-forget here — errors will be unhandled rejections and the caller gets no result. Even for an example, this teaches incorrect usage.
♻️ Proposed fix
-export function callApi() {
- api.fetch({
+export async function callApi() {
+ return api.fetch({
action: "LIST",
});
}📝 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.
| export function callApi() { | |
| api.fetch({ | |
| action: "LIST", | |
| }); | |
| } | |
| export async function callApi() { | |
| return api.fetch({ | |
| action: "LIST", | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@core/typed-nocodb-api/example/index.js` around lines 18 - 22, The callApi
function currently calls the async api.fetch without awaiting or returning its
promise, causing unhandled rejections and no result propagation; fix by making
callApi async (add async to its declaration) and either await the call (const
res = await api.fetch(...); return res;) or directly return the promise (return
api.fetch(...);) so callers receive the result and errors propagate correctly —
locate the callApi function and the api.fetch invocation to apply this change.
| "dev": "tsdown --watch", | ||
| "lint": "eslint ./ --pass-on-no-patterns --no-error-on-unmatched-pattern" | ||
| }, | ||
| "dependencies": {}, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Empty dependencies object is unnecessary.
If there are no runtime dependencies, omit the field entirely to reduce noise.
Proposed fix
- "dependencies": {},
"devDependencies": {📝 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.
| "dependencies": {}, | |
| "devDependencies": { |
🤖 Prompt for AI Agents
In `@core/typed-nocodb-api/package.json` at line 26, The package.json currently
contains an empty "dependencies" object which is unnecessary; remove the
"dependencies" key entirely from core/typed-nocodb-api's package.json so the
manifest only includes fields that have values (leaving devDependencies,
scripts, name, version, etc. intact), ensuring the file remains valid JSON after
deletion.
| [](https://www.npmjs.com/package/@stephansama/typed-nocodb-api) | ||
| [](https://www.npmjs.com/package/@stephansama/typed-nocodb-api) | ||
|
|
||
| standard schema compatible nocodb api |
There was a problem hiding this comment.
Hyphenate compound modifier: "standard-schema-compatible".
Per grammar convention, compound adjectives before a noun should be hyphenated.
Proposed fix
-standard schema compatible nocodb api
+standard-schema-compatible NocoDB API📝 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.
| standard schema compatible nocodb api | |
| standard-schema-compatible NocoDB API |
🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: Use a hyphen to join words.
Context: ...nsama/typed-nocodb-api) standard schema compatible nocodb api ##### Table of co...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In `@core/typed-nocodb-api/README.md` at line 8, Update the README header phrase
"standard schema compatible nocodb api" to use a hyphenated compound adjective
and proper capitalization (e.g., "standard-schema-compatible NocoDB API") so the
compound modifier before the noun is grammatically correct; locate and replace
the exact string "standard schema compatible nocodb api" in the README.md
content with the hyphenated/capitalized version.
| export default defineConfig({ | ||
| attw: true, | ||
| dts: true, | ||
| entry: ["src/index.ts"], | ||
| exports: true, | ||
| format: ["esm", "cjs"], | ||
| publint: true, | ||
| target: "esnext", | ||
| }); |
There was a problem hiding this comment.
Missing skipNodeModulesBundle — zod will likely be bundled into the output.
Other packages in this repo (e.g., core/ai-commit-msg/build.mjs, core/multipublish/build.mjs) include skipNodeModulesBundle: true to avoid bundling node_modules dependencies. Without this, zod (a peer dependency) will be inlined into the dist, causing bloat and potential duplicate-instance issues at runtime.
Proposed fix
export default defineConfig({
attw: true,
dts: true,
entry: ["src/index.ts"],
exports: true,
format: ["esm", "cjs"],
publint: true,
+ skipNodeModulesBundle: true,
target: "esnext",
});📝 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.
| export default defineConfig({ | |
| attw: true, | |
| dts: true, | |
| entry: ["src/index.ts"], | |
| exports: true, | |
| format: ["esm", "cjs"], | |
| publint: true, | |
| target: "esnext", | |
| }); | |
| export default defineConfig({ | |
| attw: true, | |
| dts: true, | |
| entry: ["src/index.ts"], | |
| exports: true, | |
| format: ["esm", "cjs"], | |
| publint: true, | |
| skipNodeModulesBundle: true, | |
| target: "esnext", | |
| }); |
🤖 Prompt for AI Agents
In `@core/typed-nocodb-api/tsdown.config.ts` around lines 3 - 11, The tsdown
config omits skipNodeModulesBundle which causes node_modules like zod to be
bundled; update the defineConfig call in tsdown.config.ts (the object passed to
defineConfig) to add skipNodeModulesBundle: true so external
dependencies/peerDependencies (e.g., zod) are not inlined into the dist,
matching other configs such as core/ai-commit-msg/build.mjs and
core/multipublish/build.mjs.
| import storybook from "eslint-plugin-storybook"; | ||
| import testingLibrary from "eslint-plugin-testing-library"; | ||
| import eslintPluginZodX from "eslint-plugin-zod-x"; | ||
| import eslintPluginZodX from "eslint-plugin-zod"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Misleading variable name after plugin swap.
The import variable is still named eslintPluginZodX but now imports from eslint-plugin-zod (not zod-x). Rename it to match the actual package.
♻️ Proposed fix
-import eslintPluginZodX from "eslint-plugin-zod";
+import eslintPluginZod from "eslint-plugin-zod";Also update the usage on line 49:
- eslintPluginZodX.configs.recommended,
+ eslintPluginZod.configs.recommended,🤖 Prompt for AI Agents
In `@eslint.config.ts` at line 12, The imported variable name eslintPluginZodX is
misleading because the module is now "eslint-plugin-zod"; rename the import to
eslintPluginZod and update all usages (e.g., the plugin registration that
currently references eslintPluginZodX) to use eslintPluginZod instead so the
identifier matches the actual package name.
| | [remark-asciinema](core/remark-asciinema/README.md) |  |  | A remark plugin that transforms Asciinema links into embedded players or screenshots. | | ||
| | [svelte-social-share-links](core/svelte-social-share-links/README.md) |  |  | Svelte/Web component to share the current url with various social media providers | | ||
| | [typed-events](core/typed-events/README.md) |  |  | Typed events store using standard schema | | ||
| | [typed-nocodb-api](core/typed-nocodb-api/README.md) |  |  | zod nocodb api | |
There was a problem hiding this comment.
Package description is vague.
The description "zod nocodb api" is noticeably less informative than other workspace entries. Consider aligning it with the issue objective, e.g., "Typed API client for NocoDB using Zod". This description likely comes from the package's package.json description field and would need to be updated there as the source of truth.
🤖 Prompt for AI Agents
In `@README.md` at line 62, The README entry for the workspace package
[typed-nocodb-api] uses the vague description "zod nocodb api"; update the
package's source-of-truth description in the package.json for
`@stephansama/typed-nocodb-api` to a clearer phrase such as "Typed API client for
NocoDB using Zod" (or similar), then regenerate or update the README table so
the README row for typed-nocodb-api reflects that new description; locate
package.json in the core/typed-nocodb-api module and the README table row that
references typed-nocodb-api to ensure both are consistent.
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
commit: |
…ability and maintainability
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@core/auto-readme/src/args.ts`:
- Around line 59-63: This code relies on Zod internals (value.def.innerType and
the `@ts-expect-error` STE-71 markers) which is fragile; create a formal tracked
issue (e.g., GitHub/JIRA) for STE-71 and then replace the internal access in the
function using value.def.innerType with a stable approach (use Zod's public
introspection APIs or documented helpers, or add a guarded runtime fallback that
checks for supported public properties before accessing internals), update the
code around value.meta() and the `@ts-expect-error` STE-71 annotations to
reference the new ticket ID, and add unit tests that validate behavior against
the supported Zod API to prevent regressions.
In `@core/auto-readme/src/schema.ts`:
- Around line 68-69: The schema change made affectedRegexes and collapseHeadings
required, breaking configs because they were previously defaulted to empty
arrays; update the Zod schema in schema.ts to restore defaults (e.g. change
affectedRegexes: z.array(z.string()).default([]) and collapseHeadings:
z.array(z.string()).default([])) so configs missing those keys still validate
when parsed by configSchema.parse; keep the fields as complexOptions if needed
and ensure the defaults are applied before configSchema.parse is called.
In `@core/typed-nocodb-api/src/index.ts`:
- Around line 50-60: The LIST action's querySchema currently requires both
fields and sort, forcing callers to always provide them; update LIST.querySchema
so fields and sort are optional by applying .optional() to the existing fields
schema (z.array(z.string()).or(z.string())) and to the sort schema (the
z.object({...}).transform(...)). This preserves the existing types/transform but
allows callers to omit fields and/or sort when making LIST requests.
- Around line 70-73: The READ action currently uses a literal "{recordId}" in
url (`/api/v3/data/${baseId}/${tableId}/records/{recordId}`) but has no input to
supply that ID; add an inputSchema (e.g., inputSchema: z.object({ recordId:
z.union([z.string(), z.number()]) })) to the READ action and update the fetch
logic (the method that builds the request URL for actions) to substitute the
provided recordId into the path (or append it to the records URL) instead of
leaving the placeholder; update any URL construction / dispatch code that
handles other actions so path parameters are replaced for READ before making the
request.
In `@core/typed-nocodb-api/test/index.test.ts`:
- Around line 139-158: Update the test in index.test.ts so it asserts the
correct HTTP method for the DELETE action: change the expectation on the fetch
call from method: 'patch' to method: 'delete' (the test references api.fetch and
the mockFetch assertion around the `/api/v3/data/${baseId}/${tableId}/records`
call); optionally add a brief comment like `// TODO: ensure implementation uses
"delete" method` if you want to mark coordination with src/index.ts changes.
- Around line 47-93: Reorder object literal properties in the test file to
satisfy perfectionist/sort-objects: ensure keys in mockResponse and other mock
objects are alphabetized (e.g., place pageInfo before records and order nested
keys like completed before title), and update any other mocks (e.g., within
mockFetch responses and expectations) accordingly; you can run the provided
ESLint autofix command (npx eslint core/typed-nocodb-api/test/index.test.ts
--fix) or manually reorder properties in symbols like mockResponse, pageInfo,
records, and any other test mock objects to resolve the lint errors.
- Around line 10-181: Add a test for the READ action in
core/typed-nocodb-api/test/index.test.ts that calls api.fetch({ action: 'READ',
recordId: '123' }) and verifies the fetch call uses GET and includes the
recordId in the URL (e.g.
expect.stringContaining(`/api/v3/data/${baseId}/${tableId}/records/123`)); mock
fetch to return a single record JSON (matching the LIST/CREATE shapes) and
assert the returned value equals that record, which will expose the placeholder
bug in the READ URL construction in src/index.ts (look for usages of api.fetch,
action 'READ', and any code building `/records` paths).
- Around line 1-3: Reorder and alphabetize the imports: place external modules
before parent imports (import * as z from 'zod' should come before import {
createApi } from '../src/index'), and sort the named imports from 'vitest'
alphabetically (replace "describe, it, expect, vi, beforeEach, afterEach" with
"afterEach, beforeEach, describe, expect, it, vi"); ensure both the named import
list in the vitest import and the overall import statements follow the project's
perfectionist/sort rules.
- Around line 5-8: The Zod schema variable named "schema" violates the
zod/require-schema-suffix rule; rename the constant (e.g., change "schema" to
"recordSchema" or "todoSchema") and update every reference to it (references in
tests such as usages around line 19 and any imports/exports) so all occurrences
of "schema" are replaced with the new identifier (ensure test assertions and
helper calls use the new name).
| affectedRegexes: z.array(z.string()), | ||
| collapseHeadings: z.array(z.string()), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, examine the schema.ts file around the specified lines
echo "=== Examining schema.ts lines 60-135 ==="
sed -n '60,135p' core/auto-readme/src/schema.tsRepository: stephansama/packages
Length of output: 2447
🏁 Script executed:
#!/bin/bash
# Search for all uses of configSchema parsing and _configSchema in the codebase
echo "=== Uses of configSchema.parse and similar operations ==="
rg -n 'configSchema\.(parse|safeParse|unwrap)|_configSchema' --type=ts -B2 -A2Repository: stephansama/packages
Length of output: 4818
🏁 Script executed:
#!/bin/bash
# Check if there are any config files or test files that might show expected usage
echo "=== Check for config-related tests or examples ==="
fd -e 'test.ts' -e 'config.ts' -e '.config.json' -e '.config.js' | head -20Repository: stephansama/packages
Length of output: 865
🏁 Script executed:
#!/bin/bash
# Examine the config test file to see how configs are tested
echo "=== Examining config.test.ts to understand expected usage ==="
cat -n core/auto-readme/src/config.test.tsRepository: stephansama/packages
Length of output: 2768
🏁 Script executed:
#!/antml:bash
# Check what complexOptions includes
echo "=== Looking for complexOptions definition in args.ts ==="
rg -n 'complexOptions|ComplexOptions' core/auto-readme/src/args.ts -B5 -A5Repository: stephansama/packages
Length of output: 999
🏁 Script executed:
#!/bin/bash
# Check if there are any tests that validate the parsing of partial configs
echo "=== Searching for schema validation tests ==="
cat -n core/auto-readme/src/schema.test.ts 2>/dev/null || echo "No schema.test.ts found"Repository: stephansama/packages
Length of output: 133
Removing .default([]) makes these fields required — confirmed breaking change.
Previously, affectedRegexes and collapseHeadings defaulted to [] when omitted. Now they are required fields without defaults. Since these fields are marked complexOptions and cannot be passed via CLI args, any config that omits them will fail validation at runtime in config.ts line 38 when configSchema.parse(deepmerge(...)) is called.
Existing configurations missing these fields will break. Either restore the defaults:
- affectedRegexes: z.array(z.string()),
- collapseHeadings: z.array(z.string()),
+ affectedRegexes: z.array(z.string()).default([]),
+ collapseHeadings: z.array(z.string()).default([]),Or ensure a migration path for users to add these fields to their configs.
📝 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.
| affectedRegexes: z.array(z.string()), | |
| collapseHeadings: z.array(z.string()), | |
| affectedRegexes: z.array(z.string()).default([]), | |
| collapseHeadings: z.array(z.string()).default([]), |
🤖 Prompt for AI Agents
In `@core/auto-readme/src/schema.ts` around lines 68 - 69, The schema change made
affectedRegexes and collapseHeadings required, breaking configs because they
were previously defaulted to empty arrays; update the Zod schema in schema.ts to
restore defaults (e.g. change affectedRegexes: z.array(z.string()).default([])
and collapseHeadings: z.array(z.string()).default([])) so configs missing those
keys still validate when parsed by configSchema.parse; keep the fields as
complexOptions if needed and ensure the defaults are applied before
configSchema.parse is called.
| LIST: { | ||
| method: "get", | ||
| querySchema: z.object({ | ||
| fields: z.array(z.string()).or(z.string()), | ||
| sort: z | ||
| .object({ | ||
| direction: z.enum(["asc", "desc"]), | ||
| field: z.string(), | ||
| }) | ||
| .transform((input) => JSON.stringify(input)), | ||
| }), |
There was a problem hiding this comment.
LIST querySchema requires both fields and sort — should these be optional?
Both fields and sort are required in the query schema. Callers must always provide both when performing a LIST action. Typical REST list endpoints allow these to be optional. If NocoDB doesn't require them, mark them as optional:
Proposed fix
LIST: {
method: "get",
querySchema: z.object({
- fields: z.array(z.string()).or(z.string()),
- sort: z
+ fields: z.array(z.string()).or(z.string()).optional(),
+ sort: z
.object({
direction: z.enum(["asc", "desc"]),
field: z.string(),
})
- .transform((input) => JSON.stringify(input)),
+ .transform((input) => JSON.stringify(input))
+ .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.
| LIST: { | |
| method: "get", | |
| querySchema: z.object({ | |
| fields: z.array(z.string()).or(z.string()), | |
| sort: z | |
| .object({ | |
| direction: z.enum(["asc", "desc"]), | |
| field: z.string(), | |
| }) | |
| .transform((input) => JSON.stringify(input)), | |
| }), | |
| LIST: { | |
| method: "get", | |
| querySchema: z.object({ | |
| fields: z.array(z.string()).or(z.string()).optional(), | |
| sort: z | |
| .object({ | |
| direction: z.enum(["asc", "desc"]), | |
| field: z.string(), | |
| }) | |
| .transform((input) => JSON.stringify(input)) | |
| .optional(), | |
| }), |
🤖 Prompt for AI Agents
In `@core/typed-nocodb-api/src/index.ts` around lines 50 - 60, The LIST action's
querySchema currently requires both fields and sort, forcing callers to always
provide them; update LIST.querySchema so fields and sort are optional by
applying .optional() to the existing fields schema
(z.array(z.string()).or(z.string())) and to the sort schema (the
z.object({...}).transform(...)). This preserves the existing types/transform but
allows callers to omit fields and/or sort when making LIST requests.
| READ: { | ||
| method: "get", | ||
| responseSchema: z.object({ fields: schema, id: z.number() }), | ||
| url: `/api/v3/data/${baseId}/${tableId}/records/{recordId}`, |
There was a problem hiding this comment.
READ action URL contains {recordId} placeholder that is never substituted.
The URL /api/v3/data/${baseId}/${tableId}/records/{recordId} contains a literal {recordId} string. The READ action has no inputSchema or parameter to accept a record ID, so the placeholder is never replaced. This will always request the wrong endpoint.
Consider adding an input for the record ID and interpolating it into the URL at fetch time:
Proposed approach
Add an inputSchema for READ and replace the placeholder in the fetch method:
READ: {
+ inputSchema: z.object({ id: z.number() }),
method: "get",
responseSchema: z.object({ fields: schema, id: z.number() }),
- url: `/api/v3/data/${baseId}/${tableId}/records/{recordId}`,
+ url: `/api/v3/data/${baseId}/${tableId}/records`,
},Then in the fetch method, for READ, append the ID to the URL (e.g., url + "/" + id). This requires some refactoring of the URL construction logic to handle path parameters.
🤖 Prompt for AI Agents
In `@core/typed-nocodb-api/src/index.ts` around lines 70 - 73, The READ action
currently uses a literal "{recordId}" in url
(`/api/v3/data/${baseId}/${tableId}/records/{recordId}`) but has no input to
supply that ID; add an inputSchema (e.g., inputSchema: z.object({ recordId:
z.union([z.string(), z.number()]) })) to the READ action and update the fetch
logic (the method that builds the request URL for actions) to substitute the
provided recordId into the path (or append it to the records URL) instead of
leaving the placeholder; update any URL construction / dispatch code that
handles other actions so path parameters are replaced for READ before making the
request.
1 participant
Checklist
mainhave been mergedmain