From 9717acb7bb0f6254f50e24a0023add016db9a40b Mon Sep 17 00:00:00 2001 From: fudom <143608856+fudom@users.noreply.github.com> Date: Tue, 10 Dec 2024 12:19:11 +0100 Subject: [PATCH 1/8] Create type-guards.ts --- core/src/utils/type-guards.ts | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 core/src/utils/type-guards.ts diff --git a/core/src/utils/type-guards.ts b/core/src/utils/type-guards.ts new file mode 100644 index 00000000000..e5ea629954d --- /dev/null +++ b/core/src/utils/type-guards.ts @@ -0,0 +1,6 @@ +/** + * Checks input for usable number. Not NaN and not Infinite. + */ +export const isSafeNumber = (input: unknown): input is number => { + return typeof input === 'number' && !isNaN(input) && isFinite(input); +}; From 4b41bb9ce32068293edaa7f06d33fd100231f986 Mon Sep 17 00:00:00 2001 From: fudom <143608856+fudom@users.noreply.github.com> Date: Tue, 10 Dec 2024 12:29:38 +0100 Subject: [PATCH 2/8] Check for safe number --- core/src/utils/floating-point/index.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/utils/floating-point/index.ts b/core/src/utils/floating-point/index.ts index d8ed8570741..1e25ea37197 100644 --- a/core/src/utils/floating-point/index.ts +++ b/core/src/utils/floating-point/index.ts @@ -1,4 +1,7 @@ +import { isSafeNumber } from '../type-guards.ts'; + export function getDecimalPlaces(n: number) { + if (!isSafeNumber(n)) return 0; if (n % 1 === 0) return 0; return n.toString().split('.')[1].length; } @@ -36,6 +39,7 @@ export function getDecimalPlaces(n: number) { * be used as a reference for the desired specificity. */ export function roundToMaxDecimalPlaces(n: number, ...references: number[]) { + if (!isSafeNumber(n)) return 0; const maxPlaces = Math.max(...references.map((r) => getDecimalPlaces(r))); return Number(n.toFixed(maxPlaces)); } From 21e0f607ef67f1aa912d1f00bc67c48592101393 Mon Sep 17 00:00:00 2001 From: fudom <143608856+fudom@users.noreply.github.com> Date: Tue, 10 Dec 2024 12:43:12 +0100 Subject: [PATCH 3/8] Update floating-point.spec.ts --- core/src/utils/floating-point/floating-point.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/core/src/utils/floating-point/floating-point.spec.ts b/core/src/utils/floating-point/floating-point.spec.ts index 80db4138b5b..d52fbd0f9ca 100644 --- a/core/src/utils/floating-point/floating-point.spec.ts +++ b/core/src/utils/floating-point/floating-point.spec.ts @@ -11,6 +11,12 @@ describe('floating point utils', () => { const n = getDecimalPlaces(5); expect(n).toBe(0); }); + + it('should handle nullish values', () => { + expect(getDecimalPlaces(undefined as any)).toBe(0); + expect(getDecimalPlaces(null as any)).toBe(0); + expect(getDecimalPlaces(NaN as any)).toBe(0); + }); }); describe('roundToMaxDecimalPlaces', () => { @@ -18,5 +24,11 @@ describe('floating point utils', () => { const n = roundToMaxDecimalPlaces(5.12345, 1.12, 2.123); expect(n).toBe(5.123); }); + + it('should handle nullish values', () => { + expect(roundToMaxDecimalPlaces(undefined as any)).toBe(0); + expect(roundToMaxDecimalPlaces(null as any)).toBe(0); + expect(roundToMaxDecimalPlaces(NaN as any)).toBe(0); + }) }); }); From e89ce6b57aa92474c22906e0dd6669fafb60dbd3 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Thu, 6 Mar 2025 12:14:54 -0800 Subject: [PATCH 4/8] chore(helpers): moving type safety check to helpers file --- core/src/utils/floating-point/index.ts | 2 +- core/src/utils/helpers.ts | 7 +++++++ core/src/utils/type-guards.ts | 6 ------ 3 files changed, 8 insertions(+), 7 deletions(-) delete mode 100644 core/src/utils/type-guards.ts diff --git a/core/src/utils/floating-point/index.ts b/core/src/utils/floating-point/index.ts index 1e25ea37197..1ece5510c81 100644 --- a/core/src/utils/floating-point/index.ts +++ b/core/src/utils/floating-point/index.ts @@ -1,4 +1,4 @@ -import { isSafeNumber } from '../type-guards.ts'; +import { isSafeNumber } from "@utils/helpers"; export function getDecimalPlaces(n: number) { if (!isSafeNumber(n)) return 0; diff --git a/core/src/utils/helpers.ts b/core/src/utils/helpers.ts index a740ff98ac7..a005686b779 100644 --- a/core/src/utils/helpers.ts +++ b/core/src/utils/helpers.ts @@ -424,3 +424,10 @@ export const getNextSiblingOfType = (element: Element): T | n } return null; }; + +/** + * Checks input for usable number. Not NaN and not Infinite. + */ +export const isSafeNumber = (input: unknown): input is number => { + return typeof input === 'number' && !isNaN(input) && isFinite(input); +}; diff --git a/core/src/utils/type-guards.ts b/core/src/utils/type-guards.ts deleted file mode 100644 index e5ea629954d..00000000000 --- a/core/src/utils/type-guards.ts +++ /dev/null @@ -1,6 +0,0 @@ -/** - * Checks input for usable number. Not NaN and not Infinite. - */ -export const isSafeNumber = (input: unknown): input is number => { - return typeof input === 'number' && !isNaN(input) && isFinite(input); -}; From c3e822ae6d28fa000516e396e9d6bf03935cbf93 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Thu, 6 Mar 2025 12:29:45 -0800 Subject: [PATCH 5/8] fix(lint): fixing test lint --- core/src/utils/floating-point/floating-point.spec.ts | 2 +- core/src/utils/floating-point/index.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/utils/floating-point/floating-point.spec.ts b/core/src/utils/floating-point/floating-point.spec.ts index d52fbd0f9ca..d985699ba7b 100644 --- a/core/src/utils/floating-point/floating-point.spec.ts +++ b/core/src/utils/floating-point/floating-point.spec.ts @@ -29,6 +29,6 @@ describe('floating point utils', () => { expect(roundToMaxDecimalPlaces(undefined as any)).toBe(0); expect(roundToMaxDecimalPlaces(null as any)).toBe(0); expect(roundToMaxDecimalPlaces(NaN as any)).toBe(0); - }) + }); }); }); diff --git a/core/src/utils/floating-point/index.ts b/core/src/utils/floating-point/index.ts index 1ece5510c81..6f94b801296 100644 --- a/core/src/utils/floating-point/index.ts +++ b/core/src/utils/floating-point/index.ts @@ -1,4 +1,4 @@ -import { isSafeNumber } from "@utils/helpers"; +import { isSafeNumber } from '@utils/helpers'; export function getDecimalPlaces(n: number) { if (!isSafeNumber(n)) return 0; From e01c7d728575fcbc03d7371acbcf1071e735dbbc Mon Sep 17 00:00:00 2001 From: ShaneK Date: Fri, 7 Mar 2025 11:36:42 -0800 Subject: [PATCH 6/8] fix(range): handling the case where range would return NaN if max or min were set to undefined by setting max and min to their default values if you try to set them directly to undefined --- core/src/components/range/range.tsx | 18 +++++++++++++++--- core/src/components/range/test/range.spec.ts | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index da4c2d75344..830e185a5ed 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -2,7 +2,7 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; import { Component, Element, Event, Host, Prop, State, Watch, h } from '@stencil/core'; import { findClosestIonContent, disableContentScrollY, resetContentScrollY } from '@utils/content'; import type { Attributes } from '@utils/helpers'; -import { inheritAriaAttributes, clamp, debounceEvent, renderHiddenInput } from '@utils/helpers'; +import { inheritAriaAttributes, clamp, debounceEvent, renderHiddenInput, isSafeNumber } from '@utils/helpers'; import { printIonWarning } from '@utils/logging'; import { isRTL } from '@utils/rtl'; import { createColorClasses, hostContext } from '@utils/theme'; @@ -109,7 +109,11 @@ export class Range implements ComponentInterface { */ @Prop() min = 0; @Watch('min') - protected minChanged() { + protected minChanged(newValue: number) { + if (!isSafeNumber(newValue)) { + this.min = 0; + } + if (!this.noUpdate) { this.updateRatio(); } @@ -120,7 +124,11 @@ export class Range implements ComponentInterface { */ @Prop() max = 100; @Watch('max') - protected maxChanged() { + protected maxChanged(newValue: number) { + if (!isSafeNumber(newValue)) { + this.max = 100; + } + if (!this.noUpdate) { this.updateRatio(); } @@ -300,6 +308,10 @@ export class Range implements ComponentInterface { } this.inheritedAttributes = inheritAriaAttributes(this.el); + // If the min or max is not safe, set it to 0 or 100 respectively. + // Our watch does this, but not before the initial load. + this.min = isSafeNumber(this.min) ? this.min : 0; + this.max = isSafeNumber(this.max) ? this.max : 100; } componentDidLoad() { diff --git a/core/src/components/range/test/range.spec.ts b/core/src/components/range/test/range.spec.ts index 8698f9bbf06..557d09814d0 100644 --- a/core/src/components/range/test/range.spec.ts +++ b/core/src/components/range/test/range.spec.ts @@ -28,6 +28,23 @@ describe('Range', () => { }); }); + it('should handle undefined min and max values by falling back to defaults', async () => { + const page = await newSpecPage({ + components: [Range], + html: ` +
Range
+
`, + }); + + const range = page.body.querySelector('ion-range')!; + // Here we have to cast this to any, but in its react wrapper it accepts undefined as a valid value + range.min = undefined as any; + range.max = undefined as any; + await page.waitForChanges(); + expect(range.min).toBe(0); + expect(range.max).toBe(100); + }); + it('should return the clamped value for a range dual knob component', () => { sharedRange.min = 0; sharedRange.max = 100; From 5abf5ef75249311133f15d804a818ba90ceab2ce Mon Sep 17 00:00:00 2001 From: ShaneK Date: Fri, 7 Mar 2025 13:50:41 -0800 Subject: [PATCH 7/8] fix(range): fixing case where step would break if set to undefined --- core/src/components/range/range.tsx | 7 +++++++ core/src/components/range/test/range.spec.ts | 2 ++ 2 files changed, 9 insertions(+) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 830e185a5ed..2dbb46b12f9 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -159,6 +159,12 @@ export class Range implements ComponentInterface { * Specifies the value granularity. */ @Prop() step = 1; + @Watch('step') + protected stepChanged(newValue: number) { + if (!isSafeNumber(newValue)) { + this.step = 1; + } + } /** * If `true`, tick marks are displayed based on the step value. @@ -312,6 +318,7 @@ export class Range implements ComponentInterface { // Our watch does this, but not before the initial load. this.min = isSafeNumber(this.min) ? this.min : 0; this.max = isSafeNumber(this.max) ? this.max : 100; + this.step = isSafeNumber(this.step) ? this.step : 1; } componentDidLoad() { diff --git a/core/src/components/range/test/range.spec.ts b/core/src/components/range/test/range.spec.ts index 557d09814d0..0d83082d399 100644 --- a/core/src/components/range/test/range.spec.ts +++ b/core/src/components/range/test/range.spec.ts @@ -40,9 +40,11 @@ describe('Range', () => { // Here we have to cast this to any, but in its react wrapper it accepts undefined as a valid value range.min = undefined as any; range.max = undefined as any; + range.step = undefined as any; await page.waitForChanges(); expect(range.min).toBe(0); expect(range.max).toBe(100); + expect(range.step).toBe(1); }); it('should return the clamped value for a range dual knob component', () => { From 90e26f487b631f68aa110765f2b2aa36723331f1 Mon Sep 17 00:00:00 2001 From: Shane Date: Fri, 7 Mar 2025 14:09:02 -0800 Subject: [PATCH 8/8] Update core/src/components/range/range.tsx Co-authored-by: Brandy Smith --- core/src/components/range/range.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/components/range/range.tsx b/core/src/components/range/range.tsx index 2dbb46b12f9..b36617ad3e1 100644 --- a/core/src/components/range/range.tsx +++ b/core/src/components/range/range.tsx @@ -314,8 +314,8 @@ export class Range implements ComponentInterface { } this.inheritedAttributes = inheritAriaAttributes(this.el); - // If the min or max is not safe, set it to 0 or 100 respectively. - // Our watch does this, but not before the initial load. + // If min, max, or step are not safe, set them to 0, 100, and 1, respectively. + // Each watch does this, but not before the initial load. this.min = isSafeNumber(this.min) ? this.min : 0; this.max = isSafeNumber(this.max) ? this.max : 100; this.step = isSafeNumber(this.step) ? this.step : 1;