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

error with mapped source elements #1

Open
violinchris opened this issue Jul 4, 2019 · 1 comment
Open

error with mapped source elements #1

violinchris opened this issue Jul 4, 2019 · 1 comment
Labels
bug Something isn't working

Comments

@violinchris
Copy link

i'm using this with multiple source components, like the "Responsive images" example in the readme, and i get an error. your example in the readme actually works

    return (
      <Pic placeholder={<img src="https://via.placeholder.com/500x60?text=placeholder" />}>
        <source media="(max-width: 480px)" src="/my-image-480w.jpg" />
        <source media="(max-width: 720px)" src="/my-image-720w.jpg" />
        <source media="(max-width: 1024px)" src="/my-image-1024w.jpg" />
        <img src="/my-image-high-res.jpg" />
      </Pic>      
    )

but creating source elements by mapping an array does not.

    return (
      <Pic placeholder={<img src="https://via.placeholder.com/500x60?text=placeholder" />}>
        {[480, 720, 1024].map(width => {
          return <source media={`(max-width: ${width}px)`} src={`/my-image-${width}w.jpg`} />
        })}
        <img src="/my-image-high-res.jpg" />
      </Pic>      
    )

i get the error at
image.src = loadingImage.src || loadingImage.srcSet;
TypeError: Cannot read property 'src' of undefined

i haven't figured out everything yet, but i think the problem is, ironically, because of the mysteries of the children prop, defaultImage becomes an array of the 3 source elements, and sourceImages becomes and empty array.

adding

    let childs = React.Children.toArray(children)
    defaultImage = childs.find(child => child.type !== 'source');
    sourceImages = childs.filter(child => child.type === 'source');
 

after

if (Array.isArray(children)) {

makes everything work for me, but i'm not sure what the best solution is.


Perhaps a sidenote, but this (putting both the individual source elements and the map) actually seems to work. I don't know why, because defaultImage seems to be the same array in either case.

    return (
      <Pic placeholder={<img src="https://via.placeholder.com/500x60?text=placeholder" />}>
        <source media="(max-width: 480px)" src="/my-image-480w.jpg" />
        <source media="(max-width: 720px)" src="/my-image-720w.jpg" />
        <source media="(max-width: 1024px)" src="/my-image-1024w.jpg" />
        {[480, 720, 1024].map(width => {
          return <source media={`(max-width: ${width}px)`} src={`/my-image-${width}w.jpg`} />
        })}
        <img src="/my-image-high-res.jpg" />
      </Pic>      
    )

Too tired to figure it out unless necessary, but it probably has something to do with this line,

if (!isInViewport || state === STATES.LOADED) return;

Anyway, it seems a straightforward fix, but i haven't given it careful thought or any sort of testing. thanks for the library.

@jxom
Copy link
Contributor

jxom commented Jul 8, 2019

Thanks for noting this down! I haven't really tested with dynamic elements yet, but will have a look into this issue.

@jxom jxom added the bug Something isn't working label Jul 8, 2019
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

2 participants