fix(functions): update typescript templates to support ts 6 and fix linting - #10939
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the default TypeScript functions template by commenting out unused imports, upgrading ESLint and TypeScript ESLint devDependencies, and enabling allowJs in tsconfig.dev.json. The reviewer suggested adding "noEmit": true to tsconfig.dev.json to prevent accidental compilation output during linting or IDE analysis.
| "compilerOptions": { | ||
| "allowJs": true | ||
| }, |
There was a problem hiding this comment.
Since tsconfig.dev.json is used specifically for linting configuration files and does not extend the main tsconfig.json, running tsc against this configuration (or IDE tooling doing so automatically) could accidentally emit compiled JavaScript files into the project root. Adding "noEmit": true prevents any accidental compilation output.
"compilerOptions": {
"allowJs": true,
"noEmit": true
},| @@ -1,4 +1,7 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "allowJs": true | |||
There was a problem hiding this comment.
This is the only bit from Jetski that I'm a little skeptical of but it says it's necessary to not get an error when in turn using .eslintrc.js below.
|
/joe-review |
joehan
left a comment
There was a problem hiding this comment.
⚠️ Disclaimer: This draft review was generated by an experimental AI review agent. Please verify all findings before acting on them.\n\n## Code Review Summary:firebase/firebase-tools\n\n### 🟢 Strengths & LGTM Aspects\n- Support for TypeScript 6: Upgrades ESLint parser and plugin packages to^8.0.0in the templates to resolve compatibility issues with TypeScript 6 devDependencies.\n- Zero-error Initialization UX: Resolves out-of-the-box linting warnings by commenting out unused imports (onRequest,logger) in the defaultindex.tstemplate, and fixes a style violation by removing spaces insidesetGlobalOptionsarguments.\n- Elegant Lint Config Solution: Resolves theTS18003: No inputs were founderror by introducing\"allowJs\": trueinside the development-onlytsconfig.dev.jsonwhich is target-tailored for linting.eslintrc.jswithout compromising the production TypeScript build constraints.\n\n### 🔴 Overview of Findings\n- MissingnoEmitconstraint: Intsconfig.dev.json, enabling\"allowJs\": truewithout\"noEmit\": truemight cause build systems or IDEs to accidentally emit compilation artifacts for.eslintrc.js. We recommend adding\"noEmit\": trueinside the compiler options.\n
| // In the v1 API, each function can only serve one request per container, so | ||
| // this will be the maximum concurrent request count. | ||
| setGlobalOptions({ maxInstances: 10 }); | ||
| setGlobalOptions({maxInstances: 10}); |
There was a problem hiding this comment.
🟢 [Style & Linting] Spacing\n\nFormatting setGlobalOptions parameters without inner spacing correctly aligns the code with standard Google ESLint configuration (object-curly-spacing: never) that is extended by the template's config.
| @@ -1,4 +1,7 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "allowJs": true | |||
There was a problem hiding this comment.
🔴 [TypeScript Config] Add noEmit: true\n\nRationale: Since allowJs: true is now enabled in tsconfig.dev.json, there is a risk that running the TypeScript compiler against this config will emit build artifacts. Adding \"noEmit\": true prevents this.\n\nSuggested Fix:\njson\n \"compilerOptions\": {\n \"allowJs\": true,\n \"noEmit\": true\n },\n
…inting ### Description Updates the TypeScript Cloud Functions template to support TypeScript 6 and resolves out-of-the-box linting errors/warnings in newly initialized projects. Specifically: - Upgrades `@typescript-eslint/eslint-plugin` and `@typescript-eslint/parser` to `^8.0.0` and `eslint` to `^8.57.0` in package templates to support TypeScript 6. - Adds `allowJs: true` to `tsconfig.dev.json` to resolve the `TS18003: No inputs were found` error when linting config files (like `.eslintrc.js`). - Comments out unused imports (`onRequest`, `logger`) in `index.ts` by default to avoid unused variable warnings. - Fixes spacing in `setGlobalOptions` in `index.ts` to match standard ESLint spacing rules. ### Scenarios Tested - Initialized a new project using the updated templates and verified that `npm run lint` passes successfully without any warnings or errors. - Verified that `firebase deploy functions` runs successfully without warnings or errors. ### Sample Commands - `firebase init` TAG=agy CONV=e55af4de-d0de-4b48-841c-43e04476766c
c652375 to
0e72a97
Compare
Description
Updates the TypeScript Cloud Functions template to support TypeScript 6 and resolves out-of-the-box linting errors/warnings in newly initialized projects.
Specifically:
@typescript-eslint/eslint-pluginand@typescript-eslint/parserto^8.0.0andeslintto^8.57.0in package templates to support TypeScript 6.allowJs: truetotsconfig.dev.jsonto resolve theTS18003: No inputs were founderror when linting config files (like.eslintrc.js).onRequest,logger) inindex.tsby default to avoid unused variable warnings.setGlobalOptionsinindex.tsto match standard ESLint spacing rules.Scenarios Tested
npm run lintpasses successfully without any warnings or errors.firebase deploy functionsruns successfully without warnings or errors.Sample Commands
firebase initTAG=agy
CONV=e55af4de-d0de-4b48-841c-43e04476766c