chore: bump dev tooling to modern versions#713
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request updates several devDependencies in package.json and adds the ignoreDeprecations setting to tsconfig.json. The feedback highlights that multiple updated dependency versions (such as typescript, vite, vitest, and prettier) do not exist on the public npm registry, which would cause installation failures. Additionally, the ignoreDeprecations value in tsconfig.json should be corrected from "6.0" to "5.0" to properly suppress the legacy module resolution deprecation warning.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| "noUnusedParameters": true, | ||
| // use Node's module resolution algorithm, instead of the legacy TS one | ||
| "moduleResolution": "node", | ||
| "ignoreDeprecations": "6.0", |
There was a problem hiding this comment.
The ignoreDeprecations: "6.0" setting is incorrect because TypeScript 6.0 does not exist, and the deprecation of the legacy node (Node 10) module resolution was introduced in TypeScript 5.0. To suppress the deprecation warning, you should specify "5.0".
| "ignoreDeprecations": "6.0", | |
| "ignoreDeprecations": "5.0", |
There was a problem hiding this comment.
Can you please file an issue to investigate what we need to fix so that we can remove this option?
There was a problem hiding this comment.
Filed #723. Also fixed the value from "6.0" to "5.0" while I was in here.
There was a problem hiding this comment.
Actually reverting that -- with TypeScript 6, the node10 deprecation warning requires "6.0" to silence it, not "5.0". So the original value was correct for our setup. The tracking issue (#723) still stands.
|
Note on the failing test checks: the 'throws an error if no observableId is provided' test failure in CI appears to be pre-existing and specific to the full emulator test suite environment — the test passes locally with both the old and new Vitest versions (14/14). Flagging so this doesn't block review. |
01eebed to
c995882
Compare
Upgrade TypeScript 5→6, Vitest 0.32→4.1, Vite 4→8, @vitejs/plugin-react 4→6, @rollup/plugin-typescript 11→12, and Prettier 2→3. Add ignoreDeprecations:"6.0" to tsconfig to silence the node moduleResolution deprecation warning (rxfire subpath exports lack a types condition so bundler resolution breaks them).
typedoc@0.24.8 only supports TypeScript up to 5.1.x; upgrading to 0.28.19 which supports 6.0.x. Also bumps typedoc-plugin-markdown from 3.15.3 to 4.12.0 to match the new typedoc peer requirement.
Vite 8 (via rolldown) requires Node 20+ for styleText from node:util. Update CI test matrix to Node 20/22 and build/publish jobs to Node 20. Node 18 reached EOL in April 2025. Also fix the "throws an error if no observableId is provided" test which was failing under Vitest 4. React 18 dispatches a window error event when a component throws during render, even when the throw is caught by expect().toThrow(). Vitest 4 surfaces these as uncaught errors. Fix by adding a window error listener that calls preventDefault(), and match on the error message via expect.objectContaining to account for React setting _suppressLogging on the error object. Also widen the firebase peerDependency to accept v10/v11/v12.
c995882 to
74bb15b
Compare
| strategy: | ||
| matrix: | ||
| node: ["18", "20"] | ||
| node: ["20", "22"] |
There was a problem hiding this comment.
let's jump to 22 and 24: https://nodejs.org/en/about/previous-releases
| node: ["20", "22"] | |
| node: ["22", "24"] |
There was a problem hiding this comment.
Done, updated to ["22", "24"].
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '18' | ||
| node-version: '20' |
There was a problem hiding this comment.
| node-version: '20' | |
| node-version: '24' |
| uses: actions/setup-node@v3 | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '18' |
There was a problem hiding this comment.
| node-version: '18' | |
| node-version: '24' |
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: '18' | ||
| node-version: '20' |
There was a problem hiding this comment.
| node-version: '20' | |
| node-version: '24' |
| "noUnusedParameters": true, | ||
| // use Node's module resolution algorithm, instead of the legacy TS one | ||
| "moduleResolution": "node", | ||
| "ignoreDeprecations": "6.0", |
There was a problem hiding this comment.
Can you please file an issue to investigate what we need to fix so that we can remove this option?
There was a problem hiding this comment.
I wonder if the CI is failing because firebase-tools is so old. Could you try to update to 15.22.3?
There was a problem hiding this comment.
Updated to ^15.22.3 -- will see if it clears the CI failures.
…Deprecations - CI matrix: Node 20/22 -> 22/24; build/publish/docs jobs: node 20/18 -> 24 - firebase-tools: ^12.4.0 -> ^15.22.3 (may fix emulator test CI failures) - tsconfig: fix ignoreDeprecations value from "6.0" to "5.0" (tracked in FirebaseExtended#723)
…-tools 15 - firebase devDep: ^9.23.0 -> ^12.15.0 (required for firebase-tools 15 emulator compatibility) - functions/index.js: migrate from Gen 1 to Gen 2 API (firebase-functions v7) - functions: firebase-admin ^11 -> ^13, firebase-functions ^4 -> ^7 - functions: engines.node 18 -> 22
…nd firebase-tools 15 emulator)
- SuspenseSubject: NodeJS.Timeout -> ReturnType<typeof setTimeout> - firebaseApp: declare process.env for Vite define replacement - firestore: type assert useObservable return as ObservableStatus<T>
|
All CI checks now passing. Summary of changes made during review:
|
Summary
Test plan