Bump the dev dependency for firebase-functions - #335
Conversation
This breaks tests for functions.config() since they were removed in 7.0. The tests are left in case we actually care to go back and test them with a manual older version of firebase-functions. The production code is left because the peer dep supports older versions of the functions SDK.
![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 firebase-functions to version 7.3.0 and comments out the mockConfig tests since runtime configuration is no longer supported in the new SDK version. It also introduces skipLibCheck: true in the TypeScript configurations and adds @types/cors and @types/express updates. The review feedback highlights a version mismatch where @types/express was upgraded to version 5 while firebase-functions still relies on version 4, which likely forced the use of skipLibCheck. Aligning @types/express to ^4.17.21 and removing skipLibCheck is recommended. Additionally, @types/cors should be moved from production dependencies to devDependencies unless its types are part of the public API.
| "devDependencies": { | ||
| "@types/chai": "~4.2.4", | ||
| "@types/express": "4.17.8", | ||
| "@types/express": "^5.0.6", |
There was a problem hiding this comment.
This version of @types/express (^5.0.6) is for Express 5, but firebase-functions@7.3.0 (which is also being updated in this PR) depends on Express 4 and its corresponding types (@types/express@^4.17.21). This version mismatch is the likely reason skipLibCheck: true was added to the tsconfig.json files, which is not ideal as it can hide other potential type issues.
To ensure type compatibility and allow for the removal of skipLibCheck, please align this dependency with what firebase-functions requires.
| "@types/express": "^5.0.6", | |
| "@types/express": "^4.17.21", |
| "target": "es6", | ||
| "typeRoots": ["node_modules/@types"] | ||
| "typeRoots": ["node_modules/@types"], | ||
| "skipLibCheck": true |
There was a problem hiding this comment.
This flag should be removed after fixing the @types/express version mismatch in package.json. Using skipLibCheck can hide important type-related issues in your dependencies.
| "target": "es6", | ||
| "typeRoots": ["node_modules/@types"] | ||
| "typeRoots": ["node_modules/@types"], | ||
| "skipLibCheck": true |
There was a problem hiding this comment.
This flag should be removed after fixing the @types/express version mismatch in package.json. Using skipLibCheck can hide important type-related issues in your dependencies.
| }, | ||
| "homepage": "https://github.com/firebase/firebase-functions-test#readme", | ||
| "dependencies": { | ||
| "@types/cors": "^2.8.19", |
There was a problem hiding this comment.
The @types/cors package has been added as a production dependency. Typically, @types packages are only needed for development and should be placed in devDependencies, unless your library exposes types from the cors package in its public API. If that's not the case, please move this to devDependencies to avoid adding unnecessary dependencies for consumers of this package.
2 participants
This breaks tests for functions.config() since they were removed in 7.0. The tests are left in case we actually care to go back and test them with a manual older version of firebase-functions. The production code is left because the peer dep supports older versions of the functions SDK.