davidjerleke / embla-carousel

A lightweight carousel library with fluid motion and great swipe precision.
https://www.embla-carousel.com
MIT License
6.22k stars 187 forks source link

[Bug]: TypeError: Cannot read properties of undefined (reading 'map') in AutoHeight #1009

Closed suraj776 closed 1 month ago

suraj776 commented 1 month ago

Which variants of Embla Carousel are you using?

Steps to reproduce

I'm trying to create a integration test using React Testing library.

Error:- I'm getting.

_detail: TypeError: Cannot read properties of undefined (reading 'map') at map (/node_modules/embla-carousel-auto-height/src/components/AutoHeight.ts:51:8) at highestInView (/nodemodules/embla-carousel-auto-height/src/components/AutoHeight.ts:56:48)

I tried all these mocks in my file. it didn't work. Also tried mocking the autoheight, then emblaCrousel started giving error.

https://github.com/davidjerleke/embla-carousel/tree/master/packages/embla-carousel/src/__tests__/mocks

Expected Behavior

It should be mockable when applying mocks.

Additional Context

I have also tried...

What browsers are you seeing the problem on?

All browsers

Version

^8.2.0

CodeSandbox

No response

Before submitting

davidjerleke commented 1 month ago

@suraj776 have you created the necessary mocks? See:

suraj776 commented 1 month ago

@davidjerleke yes, i do have all the mocks. Still getting error and it is from AutoHeight plugin.If i remove plugin, test cases do work.

davidjerleke commented 1 month ago

Thanks for clarifying @suraj776. I guess we need a null check here in the code.

suraj776 commented 1 month ago

@davidjerleke actually here, here you need to add null check before map.

davidjerleke commented 1 month ago

@suraj776 no here, because we're mapping the selectedIndexes variable which is undefined. We could do:

- const selectedIndexes = slideRegistry[emblaApi.selectedScrollSnap()]
+ const selectedIndexes = slideRegistry[emblaApi.selectedScrollSnap()] || []

But that might lead to errors in other places. Like the function that depends on the highestInView() function:

function setContainerHeight(): void {
  emblaApi.containerNode().style.height = `${highestInView()}px`
}

Can you create a CodeSandbox that reproduces the problem? If you do I can solve it faster.

suraj776 commented 1 month ago

@davidjerleke

Please check this sandbox code. https://codesandbox.io/p/devbox/elegant-moser-forked-psvcn3

run command

npm run test

davidjerleke commented 1 month ago

@suraj776 thanks. That helps. So this solves it:

function highestInView(): number | null {
  const { slideRegistry } = emblaApi.internalEngine()
  const selectedIndexes = slideRegistry[emblaApi.selectedScrollSnap()]

  if (!selectedIndexes) return null // Bail here if no selectedIndexes are found

  return selectedIndexes
    .map((index) => slideHeights[index])
    .reduce((a, b) => Math.max(a, b), 0)
}

function setContainerHeight(): void {
  const height = highestInView()
  if (height === null) return // Bail here if we don't have any height to apply

  emblaApi.containerNode().style.height = `${highestInView()}px`
}

Feel free to create a PR with the solution above.

Best, David

suraj776 commented 1 month ago

@davidjerleke, Thanks for quick support.

davidjerleke commented 2 weeks ago

@suraj776 I just released a bug fix for this in v8.3.1.