Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Components: Fix TS types for isValueDefined()/isValueEmpty() #43983

Merged
merged 2 commits into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

### Internal

- Fix TypeScript types for `isValueDefined()` and `isValueEmpty()` utility functions ([#43983](https://github.com/WordPress/gutenberg/pull/43983)).
- `RadioControl`: Clean up styles to use less custom CSS ([#43868](https://github.com/WordPress/gutenberg/pull/43868)).
- Remove unused `normalizeArrowKey` utility function ([#43640](https://github.com/WordPress/gutenberg/pull/43640/)).
- `SearchControl`: Convert to TypeScript ([#43871](https://github.com/WordPress/gutenberg/pull/43871)).
Expand Down
14 changes: 7 additions & 7 deletions packages/components/src/number-control/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,13 @@ function UnforwardedNumberControl(
const isStepAny = step === 'any';
const baseStep = isStepAny ? 1 : ensureNumber( step );
const baseValue = roundClamp( 0, min, max, baseStep );
const constrainValue = ( value: number, stepOverride?: number ) => {
const constrainValue = (
value: number | string,
stepOverride?: number
) => {
// When step is "any" clamp the value, otherwise round and clamp it.
return isStepAny
? Math.min( max, Math.max( min, value ) )
? Math.min( max, Math.max( min, ensureNumber( value ) ) )
: roundClamp( value, min, max, stepOverride ?? baseStep );
};

Expand Down Expand Up @@ -91,18 +94,15 @@ function UnforwardedNumberControl(
}

if ( type === inputControlActionTypes.PRESS_UP ) {
// @ts-expect-error TODO: isValueEmpty() needs to be typed properly
nextValue = add( nextValue, incrementalValue );
}

if ( type === inputControlActionTypes.PRESS_DOWN ) {
// @ts-expect-error TODO: isValueEmpty() needs to be typed properly
nextValue = subtract( nextValue, incrementalValue );
}

// @ts-expect-error TODO: Resolve discrepancy between `value` types in InputControl based components
nextState.value = constrainValue(
// @ts-expect-error TODO: isValueEmpty() needs to be typed properly
nextValue,
enableShift ? incrementalValue : undefined
);
Expand Down Expand Up @@ -151,7 +151,7 @@ function UnforwardedNumberControl(

// @ts-expect-error TODO: Resolve discrepancy between `value` types in InputControl based components
nextState.value = constrainValue(
// @ts-expect-error TODO: isValueEmpty() needs to be typed properly
// @ts-expect-error TODO: Investigate if it's ok for currentValue to be undefined
add( currentValue, distance ),
enableShift ? modifier : undefined
);
Expand All @@ -171,7 +171,7 @@ function UnforwardedNumberControl(
// @ts-expect-error TODO: Resolve discrepancy between `value` types in InputControl based components
nextState.value = applyEmptyValue
? currentValue
: // @ts-expect-error TODO: isValueEmpty() needs to be typed properly
: // @ts-expect-error TODO: Investigate if it's ok for currentValue to be undefined
constrainValue( currentValue );
}

Expand Down
8 changes: 4 additions & 4 deletions packages/components/src/utils/math.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ export function clamp( value, min, max ) {
/**
* Clamps a value based on a min/max range with rounding
*
* @param {number} value The value.
* @param {number} min The minimum range.
* @param {number} max The maximum range.
* @param {number} step A multiplier for the value.
* @param {number | string} value The value.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function can safely accept string number values because they get casted to number.

* @param {number} min The minimum range.
* @param {number} max The maximum range.
* @param {number} step A multiplier for the value.
*
* @return {number} The rounded and clamped value.
*/
Expand Down
10 changes: 4 additions & 6 deletions packages/components/src/utils/values.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
*
* @template T
*
* @param {T | null | undefined} value The value to check.
* @return {value is T} Whether value is not null or undefined.
* @param {T} value The value to check.
* @return {value is Exclude<T, null | undefined>} Whether value is not null or undefined.
*/
export function isValueDefined( value ) {
return value !== undefined && value !== null;
Expand All @@ -16,10 +16,8 @@ export function isValueDefined( value ) {
/**
* Determines if a value is empty, null, or undefined.
*
* @template T
*
* @param {T | "" | null | undefined} value The value to check.
* @return {value is T} Whether value is empty.
* @param {string | number | null | undefined} value The value to check.
* @return {value is ("" | null | undefined)} Whether value is empty.
*/
export function isValueEmpty( value ) {
const isEmptyString = value === '';
Expand Down