Conversation
![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 updates several Firebase Cloud Functions to use modular imports from the Firebase Admin SDK (e.g., importing from 'firebase-admin/app' and 'firebase-admin/auth' instead of the monolithic 'firebase-admin' package). It also enables JavaScript type checking ('checkJs': true) in the TypeScript configuration template, adding JSDoc type imports and a Mocha reference directive to support this. Regarding feedback, calling initializeApp() without arguments in the Remote Config sample will prevent app.options.projectId from being populated, leading to a runtime failure when constructing the Remote Config API URL. A suggestion has been provided to explicitly pass the project ID from the environment.
- Migrate legacy admin namespace calls (admin.auth(), admin.database(), admin.firestore(), admin.storage(), admin.messaging(), admin.machineLearning(), admin.credential) to modular Firebase Admin SDK imports. - Update elasticsearch and typesense client configurations in fulltext-search-firestore. - Replace lodash require with Object.assign in image-maker clock.js. - Correct bad-words named Filter import in text-moderation. - Fix Stripe constructor import and apiVersion in stripe sample. - Add missing declarations in instagram-auth and linkedin-auth.
…hCasesInSwitch, allowUnreachableCode, allowUnusedLabels)
- Replace (video: any) with youtube_v3.Schema in Node/youtube. - Replace any[] in JSDoc for sliceIntoChunks with generic @template T in Node/instrument-with-opentelemetry. - Replace /** @type {any} */ on sharp requires with /** @type {import('sharp').SharpConstructor} */ in 1st-gen and 2nd-gen thumbnails quickstarts.
|
|
||
| //library for resizing images | ||
| const sharp = require('sharp'); | ||
| const sharp = /** @type {import('sharp').SharpConstructor} */ (/** @type {unknown} */ (require('sharp'))); |
There was a problem hiding this comment.
This seems pretty high on TypeScript magic. Is there a better way to do whatever it's doing?
| /** | ||
| * Deletes one inactive user from the list. | ||
| * @param {admin.auth.UserRecord[]} inactiveUsers | ||
| * @param {import("firebase-admin/auth").UserRecord[]} inactiveUsers |
There was a problem hiding this comment.
This is quite an awkward way to get the type and repeated below. Can it be imported once above and used throughout.
|
|
||
| // Get the templates | ||
| const responses = await Promise.all(templatePromises); | ||
| const results = responses.map((r) => r.json()); |
There was a problem hiding this comment.
Unrelated to your PR but FYI, while looking at it AI says this line isn't quite right:
const results = responses.map((r) => r.json()); maps to Promises. It should be: const results = await Promise.all(responses.map((r) => r.json()));
2 participants
#1291 accidentally flipped
checkJstofalse, which turned off type checking in all JS samples. This turns it back on, and fixes broken types in samples.