Skip to content

Range() defaulting to infinite is easy to misuse and end up with a silent infinite loop #1801

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

Closed
sergueif opened this issue Dec 4, 2020 · 3 comments · Fixed by #1967
Closed
Labels
Milestone

Comments

@sergueif
Copy link

sergueif commented Dec 4, 2020

Hey, big fan here, long time user, first time caller. First, let me say, sharp tools can be a pleasure to use, but I also believe workplace injuries should be reported.

what happened

Someone on my team wrote an innocent-looking component with the help of Range():

const SomeComponent = ({min, max}) => {
  return <div>
    <!-- more html -->
    {Range(min, max).map(num => <option value={num}>{num}</option>)}
  </div>
}

called it some place and moved on with their day.

Later, in a different context, rendering the above component resulted in an infinite loop that took over an hour to find. The cause, of course is that Range(undefined, undefined) or without arguments is infinite. Seems it would be safer if getting an infinite range required being more explicit.

I anticipate this may get shut down, cause like "read the docs, be careful, use typescript" and what not, but has anyone else been burned by this? Am I the first to get infinite Range unintentionally?

@jdeniau jdeniau added this to the 5.0 milestone Jul 19, 2021
@jdeniau
Copy link
Member

jdeniau commented Jul 19, 2021

Hi,

It is indeed in the API since Range has been released. I'm not confident enough to change that in 4.0, but should be a discussion for 5.0.

My point of view is that we should avoid infinite loop because it's probably more a bug that a feature.

It might be taken from clojurescript with has the same comportment.
Other implementation like python or SQL required an "end" value.

A possible way to achieve that is in fact to force this behavior:

Range(0, Infinity)

and either force both start and end, or force the first parameter to be defined:

Range(2, 5); // [2, 3, 4, 4]
Range(5); // [0, 1, 2, 3, 4]

This comportment will break the following case too:

Range(10) // [ 10, 11, 12, 13, ... ]

We may want to deprecate undefined end parameter and explain this.

@sergueif
Copy link
Author

That's great! Looking forward to 5.0

@jdeniau
Copy link
Member

jdeniau commented Apr 19, 2024

Fixed in 5.x (in meta for now, but will be released soon)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants