ionic-framework icon indicating copy to clipboard operation
ionic-framework copied to clipboard

bug: ion-range throws: Cannot read properties of undefined.

Open fudom opened this issue 1 year ago • 4 comments

Prerequisites

Ionic Framework Version

v8.x

Current Behavior

Since the latest Ionic version v8 (coming from Ionic 6), the ion-range throws errors.

TypeError: Cannot read properties of undefined (reading 'toString')
  at getDecimalPlaces (ion-range.entry.js:15:14)
  at ion-range.entry.js:50:57
  at Array.map (<anonymous>)
  at roundToMaxDecimalPlaces (ion-range.entry.js:50:46)
  at ratioToValue (ion-range.entry.js:666:12)
  at get valA (ion-range.entry.js:394:16)
  at Range.renderRangeSlider (ion-range.entry.js:479:22)
  at Range.render (ion-range.entry.js:615:300)

This error occurs at least on Angular. I think it's because following:

  • Set value: Call updateRatio: min/max possible undefined
  • Set min: Call updateRatio: max/value possible undefined
  • Set max: Call updateRatio: min/value possible undefined

This results in runtime error and invisible range component. It's maybe because a framework like Angular creates the attibutes for binding even if the value is not yet ready. In this case, it's undefined first. But idk. Anyway, it would be a good practice to treat undefined, wouldn't it?

Expected Behavior

Handle undefined. It's possible that one value can be undefined.

Steps to Reproduce

<ion-item *ngIf="hasRange()">
  <ion-range [(ngModel)]="value" [min]="min" [max]="max"></ion-range>
</ion-item>

All values are inputs (Input Angular decorator). The hasRange method returns true if both values (min, max) are available (not undefined) and not equal (min !== max).

In my case I created a modal with componentProps which goes to Angular component inputs.

NOTE: I already catch undefined values. It seems that this problem comes from Ionic framework.

Code Reproduction URL

https://codepen.io/fudom/pen/ZENZxdq But not really good reproducible in a playground. Just handle undefined values!

Ionic Info

Ionic Angular 8.2.3

Additional Information

fudom avatar Jun 27 '24 06:06 fudom

@averyjohnston It seems that this bug was introduced with your change in #27375. With this change the component will always throw an error if set via JS, which includes frameworks like Angular. Because you cannot set all required properties in one shot. You'll always have and should handle undefined for input. Correct me if I'm wrong. But since this bug, the ion-range is sometimes just not rendered. I have no workaround for this.

Reviewers: @liamdebeasi @sean-perkins

fudom avatar Sep 03 '24 08:09 fudom

Thank you for reporting the issue! Could you please reproduce the problem in a StackBlitz example for us? I've created one that you can use: https://stackblitz.com/edit/angular-wx2c3i?file=src%2Fapp%2Fmodal-example.component.ts

In the top left of the StackBlitz example, you can Fork the project, make the necessary changes to reproduce the error, then Save it and Share the link with me. This will help us resolve the issue. Thank you!

brandyscarney avatar Sep 03 '24 19:09 brandyscarney

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

ionitron-bot[bot] avatar Sep 03 '24 19:09 ionitron-bot[bot]

Here is a demo on Stackblitz to reproduce the error: https://stackblitz.com/edit/angular-wx2c3i-u68tct?file=src%2Fapp%2Fexample.component.html

In this case, step is undefined. This is a common value which an HTML must be able to handle this. This problem was introduced by the changes in https://github.com/ionic-team/ionic-framework/pull/27375 which now uses an external function that assumes number. See core/src/utils/floating-point. Either undefined must be handled beforehand, or in this function. Btw. it's about "nullish", means null and undefined. Maybe NaN too.

I would use a type guard like this to make sure you can safely use the value as a number.

export const isSafeNumber = (input: unknown): input is number => {
  return typeof input === 'number' && !isNaN(input) && isFinite(input);
};

Seriously, these invalid values ​​need to be handled for input. Otherwise it will throw errors like the following or causes to unexpected behavior. Btw, this is the first time in many years that I get such error behavior in Ionic.

Error: Cannot read properties of undefined (reading 'toString')

Btw, it's weird that when the modal is first opened, no error is thrown. Although it is undefined. Maybe an Angular input thing? Because of the default value. But whatever... It doesn't matter. Handling such invalid values ​​is the right solution. For my case, I'll ensure that step has an safe number in ngOnInit. But this can happen to all values (min, max, step, etc). A value like undefined should be handled as not exist. MVC frameworks like Angular bind properties to HTML attributes whose value can be "undefined". It seems that the default value of Stencil (step = 1) is overriden in this case.

fudom avatar Sep 09 '24 10:09 fudom

Again a short explanation: The ion-range (and maybe other) elements throws an error if an input is nullish (e.g. undefined). In this case, the element is not rendered. This is a mistake made by Ionic developers. HTML elements should not explode if attributes are nullish. Handle it. Mistakes happen. But please don't ignore it. But it seems like it is a proof of this framework that seems to be dead. Or no one wants to admit that they made a mistake. It's a clear regression.

fudom avatar Oct 29 '24 10:10 fudom

Thank you for the issue report, reproduction, and PR! We've merged in your PR after minor adjustment and this issue will be addressed with the next release.

ShaneK avatar Mar 07 '25 22:03 ShaneK

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

ionitron-bot[bot] avatar Apr 06 '25 23:04 ionitron-bot[bot]