Strengthen typing of Runtimes+Langauges and their support timelines.#6866
Strengthen typing of Runtimes+Langauges and their support timelines.#6866
Conversation
- Factors out Runtimes into a new file so that they can be imported into firebaseConfig.ts and have a single source of truth and prevent the error encountered in #6774 (comment) - Adds a formal concept of languages and the ability to discriminate runtimes per language - Makes all information about a runtime table driven so there can never be a runtime without all necessary metadata - Adds the ability to dynamically fetch the latest runtime for a language - Adds support timelines for all runtimes - Unifies runtime support checks across all runtimes/languages. There are now separate warnings for when deprecation is upcoming (90d), when a runtime is deprecated, and when a runtime is decommissioned.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6866 +/- ##
==========================================
+ Coverage 54.26% 54.27% +0.01%
==========================================
Files 352 353 +1
Lines 24536 24573 +37
Branches 5083 5090 +7
==========================================
+ Hits 13315 13338 +23
- Misses 10007 10022 +15
+ Partials 1214 1213 -1 ☔ View full report in Codecov by Sentry. |
src/deploy/functions/prepare.ts
Outdated
| runtime: codebaseConfig.runtime || "", | ||
| }; | ||
| const firebaseJsonRuntime = codebaseConfig.runtime; | ||
| if (firebaseJsonRuntime) { |
There was a problem hiding this comment.
Optional - prefer this as a single if statement
| if (firebaseJsonRuntime) { | |
| if (firebaseJsonRuntime && !supported.isRuntime(firebaseJsonRuntime as string)) { |
| import * as supported from "../supported"; | ||
|
|
||
| // have to require this because no @types/cjson available | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires |
There was a problem hiding this comment.
Optional, unrelated to this PR: Looks like there are types now! https://www.npmjs.com/package/@types/cjson
| const runtime = context.runtime ? context.runtime : LATEST_VERSION; | ||
| if (!runtimes.isValidRuntime(runtime)) { | ||
| throw new FirebaseError(`Runtime ${runtime} is not a valid Python runtime`); | ||
| const runtime = context.runtime ? context.runtime : supported.latest("python"); |
There was a problem hiding this comment.
Nit: context.runtime ?? supported.latest("python") is probably cleaner than a ternary
| region: "us-west1", | ||
| project: projectNumber, | ||
| runtime: "nodejs14", | ||
| runtime: "nodejs14" as const, |
There was a problem hiding this comment.
TIL that as const satisfies enums without having to import them - good to know!
There was a problem hiding this comment.
yes, "as const" will make a string literal type rather than just a string type.
3 participants
Uh oh!
There was an error while loading. Please reload this page.