Skip to content

Fix type definitions that contradict the implementation#257

Merged
sindresorhus merged 1 commit into
sindresorhus:mainfrom
patrickwehbe:fix-type-implementation-mismatches
Jun 22, 2026
Merged

Fix type definitions that contradict the implementation#257
sindresorhus merged 1 commit into
sindresorhus:mainfrom
patrickwehbe:fix-type-implementation-mismatches

Conversation

@patrickwehbe

Copy link
Copy Markdown
Contributor

While using the types, I hit two cases where the type definitions disagree with what index.js actually does.

color allows true, but the setter rejects it

color is typed as Color | boolean on both Options and the Ora interface, so color: true type-checks. At runtime the setter throws for anything that is not a valid color or false:

set color(value) {
	if (value !== undefined && value !== false && !validColors.has(value)) {
		throw new Error('The `color` option must be a valid color or `false` to disable');
	}

	this.#color = value;
}

So this compiles but throws:

ora({color: true}); // Error: The `color` option must be a valid color or `false` to disable

false is meaningful (it disables the color), but true is not a valid value. This narrows the type to Color | false in both places.

prefixText/suffixText on the Ora interface drop the function form

On the Ora interface, prefixText and suffixText are typed as string, but the implementation resolves a function by calling it:

#formatAffix(value, separator, placeBefore = false) {
	const resolved = typeof value === 'function' ? value() : value;
	...
}

The JSDoc on both members already says "text or function that returns text", the PrefixTextGenerator/SuffixTextGenerator types are already exported, and Options and PersistOptions already use string | PrefixTextGenerator / string | SuffixTextGenerator. Only the Ora interface members were left as plain string, so assigning a function to spinner.prefixText is a type error even though it works at runtime. This widens them to match.

Tests

Added tsd assertions for the rejected color: true and for the function forms of prefixText/suffixText.

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`.
@sindresorhus
sindresorhus merged commit 431ebc4 into sindresorhus:main Jun 22, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants