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

[BUG] this.step[...].posY would benefit from null checking #177

Closed
rgoldiez opened this issue Feb 21, 2022 · 5 comments
Closed

[BUG] this.step[...].posY would benefit from null checking #177

rgoldiez opened this issue Feb 21, 2022 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@rgoldiez
Copy link

I can't pinpoint the exact scenario, but this.steps can be an empty array. Most places in events.ts have null checking with typescript's ? optional chaining operator except this one:

https://github.com/roman-rr/cupertino-pane/blob/5b49dc37dbc33a67093d473053872975dc816ba6/src/events.ts#L252

We've seen a few Cannot read properties of undefined (reading 'posY') errors come through our logging framework. In reading through the code, it seems that the line above can be the only culprit. It would be good add optional chaining here, too.

@roman-rr roman-rr self-assigned this Feb 22, 2022
@roman-rr roman-rr added the bug Something isn't working label Feb 22, 2022
@roman-rr
Copy link
Collaborator

@rgoldiez Thank you for report.
Always using an optional chaining is a compromise. And here is a good litmus paper to know the exact reason of issue and fix it in correct way. So, decided to fix rather than use additional compromise with chaining.

Bug is easy to reproduce: just hold touchmove action on pane initialization and touchstart will be skipped.

Should be fixed in last commit!

@rgoldiez
Copy link
Author

@roman-rr - great find and fix! Thank you!

@EdithOrt
Copy link

I'm new to this, but how do I download the latest version with this bug fixed?

@roman-rr
Copy link
Collaborator

Hello @EdithOrt
You could install package right from git repository.

npm install git+https://github.com/roman-rr/cupertino-pane.git

@EdithOrt
Copy link

Thanks @roman-rr , it helped me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants