Fix type definitions that contradict the implementation#257
Merged
sindresorhus merged 1 commit intoJun 22, 2026
Merged
Conversation
Two of the public types allow values that the runtime does not accept, or
omit values that it does.
`color` was typed as `Color | boolean`, but the `color` setter throws for
any value that is not a valid color or `false`:
throw new Error('The `color` option must be a valid color or `false` to disable');
So `color: true` passes the type check but throws at runtime. Narrow it to
`Color | false` on both `Options` and the `Ora` interface.
`prefixText` and `suffixText` on the `Ora` interface were typed as `string`,
but `#formatAffix` calls the value when it is a function:
const resolved = typeof value === 'function' ? value() : value;
The JSDoc for both already says "text or function that returns text", the
`PrefixTextGenerator`/`SuffixTextGenerator` types are exported, and `Options`
and `PersistOptions` already use them. Widen the `Ora` interface members to
`string | PrefixTextGenerator` and `string | SuffixTextGenerator` to match.
Add type tests covering the rejected `color: true` and the function forms of
`prefixText`/`suffixText`.
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
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.
While using the types, I hit two cases where the type definitions disagree with what
index.jsactually does.colorallowstrue, but the setter rejects itcoloris typed asColor | booleanon bothOptionsand theOrainterface, socolor: truetype-checks. At runtime the setter throws for anything that is not a valid color orfalse:So this compiles but throws:
falseis meaningful (it disables the color), buttrueis not a valid value. This narrows the type toColor | falsein both places.prefixText/suffixTexton theOrainterface drop the function formOn the
Orainterface,prefixTextandsuffixTextare typed asstring, but the implementation resolves a function by calling it:The JSDoc on both members already says "text or function that returns text", the
PrefixTextGenerator/SuffixTextGeneratortypes are already exported, andOptionsandPersistOptionsalready usestring | PrefixTextGenerator/string | SuffixTextGenerator. Only theOrainterface members were left as plainstring, so assigning a function tospinner.prefixTextis a type error even though it works at runtime. This widens them to match.Tests
Added
tsdassertions for the rejectedcolor: trueand for the function forms ofprefixText/suffixText.