Safari 13.x t.addEventListener is not a function

[hamidrezahanafi]

Bug is related to

• embla-carousel (core package)

Embla Carousel version

• v8.0.0-rc11

Describe the bug

t.addEventListener is not a function on Safari 13.x
This comes adding event to mediaHandlers
mediaHandlers.add(query, 'change', reActivate));

Rest of the error:
`TypeError: t.addEventListener is not a function. (In 't.addEventListener(r,o,i)', 't.addEventListener' is undefined)

./node_modules/embla-carousel/embla-carousel.esm.js at line 192:10

function EventStore() {
let listeners = [];
function add(node, type, handler, options = {
passive: true
}) {
node.addEventListener(type, handler, options);
listeners.push(() => node.removeEventListener(type, handler, options));
return self;
}`

CodeSandbox

It only happens on older browsers

Steps to reproduce

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Expected behavior

Is it possible to support older browsers?

Additional context

• Add any other context about the problem here, like screenshots or similar.

[hamidrezahanafi]

@davidjerleke I added a PR to check if the addEventListener exist before calling it that might help just not throw errors on older browsers and work limited
#561

[davidjerleke]

Hi @hamidrezahanafi,

First of all, thank you for wanting to contribute to Embla Carousel. The intention to help improve this library is always appreciated 👍!

A friendly reminder: Before starting any work, always start a new discussion first. The reason why I'm mentioning this is because this could prevent you from doing unnecessary work. Looking at the usage statistics, it seems like iOS13 is not quite common from 2022 and forward, and as you said it's a quite old version. I'm not convinced that we should go through the trouble solving this:

Best,
David

[7iomka]

Hi everyone.
Related issue with possible solutions: microsoft/TypeScript#32210

I'm always in favour of supporting new features, but usually when it comes to complex properties like flex's gap - it requires a lot of overhead and in such cases it's worth explicitly defining the list of supported browsers on the product page to avoid wasting energy and bloating the codebase with unnecessary fallbacks.
However, in this case we have a well-known exception to the rule.

People knowing about this use either an outdated method or make a wrapper with a simple check and a small fallback.

My proposal is to add a small fallback in your helper function code - it's not difficult for you and won't add a lot of code to the bundle but unlike the current behaviour or the proposed PR - it will work everywhere.

before

node.addEventListener(type, handler, options)
listeners.push(() => node.removeEventListener(type, handler, options))

after

if ('addEventListener' in node) {
      node.addEventListener(type, handler, options)
     listeners.push(() => node.removeEventListener(type, handler, options))
} else {
     node.addListener?.(handler);
     listeners.push(() => node.removeListener?.(handler))
}

If for some reason such a condition may seem redundant to you - I'll accept even the current PR
I want to make progressive improvements, not breaking ones.

[hamidrezahanafi]

Hey @davidjerleke yeah will start the discussion first in the future Thanks for the reminder.
The solution from @7iomka also seems reasonable too. My intention is more to fail-safe instead of a hard crash. I am using it now and seeing 10/20 hard crashes per hour because of this issue. It seems that people are still using their old mac with outdated safari and they might continue doing it for years to come.
Is there any hard objection for not using one of the two simple solutions so at least not to crash?

[davidjerleke]

@7iomka thank you for joining the discussion. Thanks for the additional input @hamidrezahanafi.

I don't think that "Changing this will prevent this from crash in an old browser" is a good argument for fixing it. We can't start supporting IE11 because Embla will crash there simply because Embla doesn't support IE11 anymore.

However, since you have proof that some people (although not many based on statistics) are still using iOS13, and if it's only a matter of changing one thing to make it work:

if ('addListener' in node) {
     node.addListener(handler);
     listeners.push(() => node.removeListener(handler))
} else {
     node.addEventListener(type, handler, options)
     listeners.push(() => node.removeEventListener(type, handler, options))
}

I would be happy to add this too Embla core for now. But we need to test this on iOS13 to make sure that the above fix is enough.

Best,
David

[hamidrezahanafi]

Hey @davidjerleke Thanks for understanding
I just tested on Safari 13.1 the new code at https://github.com/davidjerleke/embla-carousel/pull/561/files
Video:

untitled_aAXrp4xh.mp4

[davidjerleke]

Thanks @hamidrezahanafi. I'll have to do some functionality testing before I can be sure that all features work in Safari 13 before merging the PR.

Best,
David

[hamidrezahanafi]

Hey @davidjerleke, Is there any update on this? Let me know how I can help with testing.

[davidjerleke]

Hi @hamidrezahanafi,

Yes that would be helpful 👍. I'm thinking that we should mainly test the following things:

• Media query list listeners: Test the breakpoints option meaning setting different options based on breakpoints. For example, you can use media queries that will trigger when you trigger an orientation cchange on your mobile phone (landscape --> portrait and portrait --> landscape).

• ResizeObserver: Make sure that changing slide sizes triggers a emblaApi.reInit() so it's working as expected. This can be tested like this.

• MutationObserver: Test if adding/removing slides triggers a emblaApi.reInit() so the carousel works as expected after that. Can be achieved with a setTimeout that adds slides or similar.

• IntersectionObserver: Test if slidesInView is working correctly as you scroll. This can be tested like this:

emblaApi.on('slidesInView', (emblaApi) => {
  console.log(emblaApi.slidesInView())
})

Any help appreciated. Please make sure to test the latest version 8.0.0-rc12.

Best,
David

[hamidrezahanafi]

Hey @davidjerleke I tested following on Safari 13
Media query list listeners works perfectly and applies all different options based on screen size
IntersectionObserver works as the slidesInView event fired while scrolling
ResizeObserver changed slide size and reInit event fired as expected
MutationObserver set a timeout and added one slide and reInit event fired as expected

[davidjerleke]

Thanks a lot for your efforts @hamidrezahanafi 👍. I've been busy rewriting all tests for this project from scratch before the stable v8 release so any help with other stuff is appreciated. This speeds up things.

I just merged your PR and will release it with version 8.0.0-rc13 as soon as possible.

[davidjerleke]

Thanks for all your help @hamidrezahanafi! I just released this feature with version 8.0.0-rc13.