added typescript definitions (intellisense) to nodejs starter #345
Conversation
WalkthroughThis PR makes two changes: cosmetic formatting edits in node/starter/src/main.js (quote style and minor spacing/param spacing) and adds node/starter/src/types.d.ts which introduces TypeScript global augmentations for interfaces Context, AppwriteRequest, and AppwriteResponse and an export {} module marker. No runtime behavior or exported API signatures were changed. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
node/starter/src/main.js (1)
3-14: Outdated inline comments conflict with the new type definitions.The comment block describes properties that don't match the
RequestandResponseinterfaces intypes.d.ts:
- Comment mentions
payloadandvariables, but theRequesttype hasbodyText,bodyJson,bodyBinary, etc.- Comment mentions
send(text, status), but theResponsetype hastext(),json(),empty(),redirect(), andbinary().Consider updating or removing these comments to avoid confusion, since IntelliSense will now provide accurate documentation from the type definitions.
-/* - 'req' variable has: - 'headers' - object with request headers - 'payload' - request body data as a string - 'variables' - object with function variables - - 'res' variable has: - 'send(text, status)' - function to return text response. Status code defaults to 200 - 'json(obj, status)' - function to return JSON response. Status code defaults to 200 - - If an error is thrown, a response with code 500 will be returned. -*/ +// See types.d.ts for Request and Response type definitions. +// If an error is thrown, a response with code 500 will be returned.node/starter/src/types.d.ts (1)
29-89: Interface namesRequestandResponseshadow Fetch API globals.In Node.js 18+ (and modern browsers),
RequestandResponseare built-in global types from the Fetch API. Declaring them indeclare globalwill shadow those types, which could cause type errors if developers use the Fetch API in their functions.Consider prefixing these interfaces to avoid conflicts:
- interface Request { + interface AppwriteRequest {- interface Response { + interface AppwriteResponse {Then update
Contextaccordingly:- req: Request; - res: Response; + req: AppwriteRequest; + res: AppwriteResponse;Also applies to: 91-144
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
node/starter/src/main.js(2 hunks)node/starter/src/types.d.ts(1 hunks)
🔇 Additional comments (4)
node/starter/src/main.js (2)
1-2: Type reference and JSDoc annotation look good.The
/// <reference path="./types.d.ts" />directive and@param {Context} contextJSDoc correctly enable IntelliSense for the destructuredreq,res,log, anderrorparameters without requiring a build step.Also applies to: 18-20
36-51: Formatting changes are consistent.The single-quote standardization aligns with common JavaScript style conventions.
node/starter/src/types.d.ts (2)
1-27: Global augmentation structure and Context interface look good.The
declare globalwithexport {}pattern correctly augments the global scope. TheContextinterface withreq,res,log, anderrormembers matches the Appwrite function signature and has helpful JSDoc documentation.Also applies to: 145-147
98-102: The response methods are correctly typed to returnvoid. Thereturn res.text(...)andreturn res.json(...)patterns in the code use the return statement for control flow (to stop execution within the handler), not to capture a return value. Appwrite's response methods "immediately produce the HTTP execution response" without returning a meaningful value; thevoidreturn type accurately reflects this behavior. This pattern is consistent across all Node.js templates in the codebase.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
node/starter/src/main.js (2)
8-12: JSDocContexttyping for the destructured parameter looks good; consider a small tweak if IntelliSense is spottyAnnotating the function with
@param {Context} contextshould allow TS/VS Code to infer types for{ req, res, log, error }from yourContextinterface. If you notice any gaps in autocomplete, an alternative is to accept a namedcontextparameter and destructure inside the function, which some tooling handles more consistently:-/** - * @param {Context} context - */ -// This Appwrite function will be executed every time your function is triggered -export default async ({ req, res, log, error }) => { +/** + * @param {Context} context + */ +// This Appwrite function will be executed every time your function is triggered +export default async (context) => { + const { req, res, log, error } = context;
26-28: Error logging is fine; optionally harden it for non-Error throws
error('Could not list users: ' + err.message);is acceptable assumingusers.list()throwsErrorinstances, but you could make this a bit more robust against unexpected values:- } catch (err) { - error('Could not list users: ' + err.message); + } catch (err) { + const message = err?.message ?? String(err); + error('Could not list users: ' + message);Not mandatory, but slightly safer for odd error shapes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
node/starter/src/main.js(2 hunks)node/starter/src/types.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- node/starter/src/types.d.ts
🔇 Additional comments (3)
node/starter/src/main.js (3)
1-5: Triple-slash reference is appropriate for wiring in the d.ts typesUsing
/// <reference path="./types.d.ts" />is a good way to pull the ambientContext/request/response types into this JS file without changing runtime behavior. Please just double-check in VS Code that the file picks up those types as expected for hover/IntelliSense, per your test plan.
31-35: Ping route handler remains clear and idiomaticThe
req.path === '/ping'check withreturn res.text('Pong');keeps the behavior simple and explicit, and the added comments make the intent clear. No issues here.
38-41: JSON response content changes are non-breakingUpdating the
motto,learn,connect, andgetInspiredstrings/URLs is purely content-level; the response shape is unchanged, so existing consumers should keep working while benefiting from the new links/text.
1 participant
What does this PR do?
This PR adds TypeScript definitions (
types.d.ts) to the Node.js starter template and updates the entry file to enable IntelliSense.Changes:
node/starter/src/types.d.tswhich defines theContext,Request, andResponseinterfaces based on the Appwrite documentation./// <reference path="./types.d.ts" />directive./** @param {Context} context */) to the exported function to properly type the destructured parameters (req,res,log,error).Reasoning:
Currently, the Node.js starter lacks IntelliSense support. By adding these definitions, developers get immediate autocomplete and type checking for the function context in VS Code (and other editors) without needing a build step or a
tsconfig.json. This significantly improves the Developer Experience (DX).Test Plan
I verified the changes manually in VS Code:
node/starterdirectory in VS Code.reqorresinside the function parameters.Request,Response) instead ofany.res.orreq.inside the function.json(),headers,bodyText).Related PRs and Issues
Closes #344
Have you read the Contributing Guidelines on issues?
Yes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.