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

TypeErrorCannot read property '0' of undefined #27

Closed
bitttttten opened this issue Oct 11, 2019 · 20 comments
Closed

TypeErrorCannot read property '0' of undefined #27

bitttttten opened this issue Oct 11, 2019 · 20 comments
Labels
bug Something isn't working resolved This issue is resolved

Comments

@bitttttten
Copy link

Hey! Let me first say, loving the library so far. Very easy to get a really nice carousel with 0 config :) So thanks for the nice defaults.

I just got this error reported in my error tracking, and it leads to embla-carousel/lib/index.

Environment:

Chrome Mobile Version 77.0.3865
Android Version 8.0.0

TypeError: Cannot read property '0' of undefined
 at ./node_modules/embla-carousel/lib/index.js(read:1589:46)
 at ./node_modules/embla-carousel/lib/index.js(apply:1417:29)
 at ./node_modules/logrocket/dist/build.umd.js(HTMLElement.o:2857:31)

chrome_2019-10-11_22-49-17

So that's

./node_modules/embla-carousel/lib/index.js in read on line 1589:46
1586 | function read(evt, axis) {
1587 | var isMouse = state.isMouse;
1588 | var c = coords[axis];
1589 | var value = isMouse ? evt[c] : evt.touches[0][c];
1590 | return pointValue.setNumber(value);
1591 | }
1592 |  
1593 | function down(evt) {
1594 | state.isMouse = !!evt.type.match(/mouse/);

./node_modules/embla-carousel/lib/index.js in apply on line 1417:29
1413 | var resist = !params.loop && reachedLimit ? 2 : 1;
1414 | target.addNumber(diff / resist);
1415 | evt.preventDefault();
1416 | } else {
1417 | var X = pointer.read(evt, 'x').get();
1418 | var Y = pointer.read(evt, 'y').get();
1419 | var diffX = Math.abs(X - startX.get());
1420 | var diffY = Math.abs(Y - startY.get());
1421 | state.preventScroll = diffX > diffY;
1422 | if (!state.preventScroll && !state.preventClick) up();

If you need some copy and paste friendly text.

Not really sure how to go from here! I was going to default value to 0 if evt.touches is not defined although that is just a hack :)

Thanks!

@davidjerleke davidjerleke added the investigating Issue is being looked into label Oct 12, 2019
@davidjerleke
Copy link
Owner

davidjerleke commented Oct 12, 2019

Hi @bitttttten,

Thank you for your kind words and taking time to report this 👍.

That's strange because I'm assuming your android is a touch device 😕? I don't know if this is a browser specific bug but we should definitely look into this.

  • Would you mind logging what you get if you log evt.type so we can determine if it’s the touchstart, touchmove or touchend event that’s causing this?
  • What do you get if you log evt? This is interesting because it doesn’t make sense for me why the touches property is missing yet.

I should probably apply for the BrowserStack open source program to be able to test this device. But unfortunately it could take some time to get the application processed. I'll let you know as soon as I'm on to something. Meanwhile, if you find any information like a relevant stackoverflow thread or similar that could point me in the right direction, please share it. Thank you for your patience!

Kindly,
David

@davidjerleke davidjerleke added the help wanted Feel free to help out label Oct 12, 2019
@bitttttten
Copy link
Author

I can't actually reproduce it myself! Also I don't have an Android 8 device. This error actually came from LogRocket which is like Sentry if you are familiar with that. So it was an error that happened on one of my sites that an error reporter tracked and reported.

I've been asking around for a device running Android 8 so I'll try to reproduce if I get my hands on one 👍

@davidjerleke
Copy link
Owner

davidjerleke commented Oct 12, 2019

Hello @bitttttten,

Thanks for getting back to me. Interesting, so at the time of writing we can’t rule out that LogRocket is doing something strange 😕.

I’ll apply for BrowserStack open source testing as soon as possible.

Thank you for helping me out with this! Let’s see what we can find 👍🏻.

Best,
David

@davidjerleke
Copy link
Owner

davidjerleke commented Oct 17, 2019

Hi @bitttttten,

Status update
I've applied for the BrowserStack open source program and I'm waiting for an approval from their side.

Kindly,
David

@bitttttten
Copy link
Author

Ah good news. Good luck! ✨

@davidjerleke
Copy link
Owner

davidjerleke commented Oct 18, 2019

Hello again @bitttttten,

Thanks. BrowserStack granted me access today and I tried to reproduce this error for an hour or so without any luck. Do you have any ideas how we can proceed with this? Maybe test ideas?

Thanks in advance.

Best,
David

@bitttttten
Copy link
Author

bitttttten commented Oct 18, 2019

Hey, good news! congrats ✨

ehh sadly not. It's just an anonymous crash report. I tried looking into it myself but it seems like a weird error. If it means anything, it was only 1 report out of 1000 users of Android 🤔

I tried to also reproduce but no luck. And I have had no body talking about it.

Maybe it's okay to close the issue and then tackle it if somebody is able to reproduce!

@davidjerleke davidjerleke removed the investigating Issue is being looked into label Oct 19, 2019
@davidjerleke
Copy link
Owner

davidjerleke commented Oct 19, 2019

Hey @bitttttten, thanks!

Yeah, it's certainly odd. I can't make sense of why the event.touches (touch list) is empty on a touchstart or touchmove event. I mean that's the whole point of touch events, to track touch pointers 😕. I will close this issue for now. Let's see if someone finds this issue and is able to reproduce it someday. Maybe that person will enlighten us 🙂.

Thank you very much for taking time to open this issue and don't hesitate to do it again if you find a bug or have a feature request.

Cheers!
David

@davidjerleke davidjerleke added the bug Something isn't working label Oct 21, 2019
@korsvanloon
Copy link

korsvanloon commented Feb 12, 2020

@davidcetinkaya I see this error as well on the error reporting of production logs for version 2.6.
For now we've upgraded the package, hope it fixes it (it's indeed hard to reproduce).
However, I think isMouse can better be implemented as evt instanceof MouseEvent or isTouch as evt instanceof TouchEvent.

@davidjerleke
Copy link
Owner

davidjerleke commented Feb 12, 2020

Hello @korsvanloon,

Thank you for your thoughts.

At the time of writing, no fix has been shipped for this issue because I haven't been able to reproduce it. It would be great if we could try to narrow it down. If the touchmove event is causing this, setting isMouse as you suggest won't help. But if it's the touchstart event, the fix you suggest would probably resolve the issue.

This issue has the help wanted label so feel free to try reproducing it. Thank you in advance.

Kindly,
David

@davidjerleke davidjerleke reopened this Feb 12, 2020
@davidjerleke
Copy link
Owner

davidjerleke commented Feb 12, 2020

@bitttttten and @korsvanloon,

I just did some testing and I finally understand why this is happening. If you perform a touchstart event and hold the touch for long enough the Android device will actually fire a mousemove event, which is interesting. I don't know if there is a sensible reason to this or if it's an Android bug. Take a look at the screenshots:

mouse_event

mouse_event_details

Embla only checks the event type on pointerdown (touchstart or mousedown) and this is why Embla fails when a drag interaction that starts with a touch event is suddenly followed by a mouse event.

I'm going to be a very happy guy as soon as I can implement pointer events instead of touch and mouse separately, as soon as usage of browsers that don't support it drops, so I don't have hack my way around this.

I'll let you know when I've released a patch version that fixes the issue.

Best,
David

@davidjerleke davidjerleke removed the help wanted Feel free to help out label Feb 12, 2020
@korsvanloon
Copy link

Great news! Thanks a bunch!

@bitttttten
Copy link
Author

How did you manage to debug that? 🙈 Nice one :)

@davidjerleke
Copy link
Owner

@korsvanloon and @bitttttten,

I'm happy to announce that patch version 2.7.4 ships with a fix for this issue.

I'd very much appreciate if you could take a moment and do some testing so I didn't crash anything else and if you could try to reproduce the issue.

Thanks in advance,
David

@davidjerleke davidjerleke added awaiting response Issue is awaiting feedback resolved This issue is resolved labels Feb 12, 2020
@davidjerleke
Copy link
Owner

Hello guys (@korsvanloon, @bitttttten),

Are you able to verify if the patch release fixes the issue?

Kindly,
David

@korsvanloon
Copy link

korsvanloon commented Feb 17, 2020

According to our testing now it seems good, but I'm waiting for the update to go live and see our error reporting for this issue to go down.
I'll come back at you when we've gone live with your update. Should be a couple of days :)

@davidjerleke
Copy link
Owner

Hi @korsvanloon,
Thank you for the update. Sounds great, looking forward to it 👍.

Best,
David

@davidjerleke
Copy link
Owner

Hello again @korsvanloon,

I'm going to close this issue due to its stale status. I'd still very much appreciate if you could verify if the patch release fixes the issue for you. It would be great to reach some kind of resolution 🙂.

Kindly,
David

@korsvanloon
Copy link

Sorry for the late response but I can confirm this is fixed :)

@davidjerleke
Copy link
Owner

Thanks @korsvanloon, cheers!

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

No branches or pull requests

3 participants