fix(types): make MetricType a string union matching runtime values - #790
Open
milcho0604 wants to merge 1 commit into
Open
fix(types): make MetricType a string union matching runtime values#790milcho0604 wants to merge 1 commit into
milcho0604 wants to merge 1 commit into
Conversation
The declaration file declares MetricType as a numeric enum, but no such
object is exported at runtime: require('prom-client').MetricType is
undefined, so value-style uses like MetricType.Counter type-check and
then throw TypeError when evaluated (prometheus#336, diagnosed in 2020). Meanwhile
the property it describes (MetricObject.type) is a runtime string:
'counter', 'gauge', 'histogram' or 'summary'.
Model it the way the neighbouring Aggregator type already models the
same situation: a string-literal union of the runtime values. A
declaration-only enum with string values would still claim a runtime
object exists, and adding a runtime object would introduce new JS API
solely to rehabilitate one that never worked - the union models what the
library actually returns. index.js is untouched. Type-position uses keep
compiling, comparisons against reported metric types now type-check and
narrow correctly (with the numeric enum, m.type === 'counter' was itself
a compile error), and value-style access fails at compile time instead
of at runtime.
Fixes prometheus#336
Signed-off-by: Changhyun Kim <milcho0604@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #336
index.d.tsdeclaresMetricTypeas a numeric enum, but no such object is exported at runtime —require('prom-client').MetricTypeisundefined, soMetricType.Countertype-checks and then throws when evaluated. Meanwhile the property it describes,MetricObject.type, is a runtime string ('counter','gauge','histogram','summary'— set in lib/counter.js, lib/gauge.js, lib/histogram.js, lib/summary.js). Both halves were diagnosed in #336 back in 2020.This models it the way the neighbouring
Aggregatortype already models the same situation — a string-literal union of the runtime values:Why not the alternatives: a declaration-only enum with string values would still claim a runtime object exists (the crash stays), and shipping a runtime object would add new JS API solely to rehabilitate one that never worked. The union just describes what the library returns.
What changes for consumers (declaration-only;
index.jsis untouched):m.type === 'counter') now type-checks and narrows — with the numeric enum this comparison was itself a compile error (TS2367, no overlap), forcing casts.MetricType.Counter) stops compiling (TS2693) — it previously threw at runtime.verbatimModuleSyntax, importingMetricTypenow requiresimport type.Tests (
test/typescript.ts, compiled bytsc --project .in CI):switchthat fails to compile if the union and the member list drift apart in either direction (verified by mutation: removing'gauge'→ TS2678, adding a member → TS2322 on theneverdefault);@ts-expect-errorguard onMetricType.Counter— if a value declaration forMetricTypeis ever introduced, the directive turns unused and the compile fails, flagging the test for update.Since Unreleased already carries breaking changes for the first release under the Prometheus umbrella, this seemed like the right window for a type-level break; the CHANGELOG entry is under Breaking.