React-infinite fails with Cannot read property 'scrollShouldBeIgnored' of undefined

[slmgc]

Hi @developit!
I was checking some functionality which is based on https://github.com/seatgeek/react-infinite. I wasn't able to check if onLoad triggers because it throws an error while scrolling. It's reproducible with preact@5.1.0-beta.26 as well as preact@5.3.0:

import Infinite from 'react-infinite'

export default class TestInfinite extends React.Component {
    onLoad = () => {
        console.log('`onLoad` should be triggered')
    }

    render() {
        const height = 300
        const offset = 100

        return (
            <Infinite
                containerHeight={height - offset}
                elementHeight={height}
                infiniteLoadBeginEdgeOffset={offset}
                onInfiniteLoad={this.onLoad}>
                    <div style={{height}}>content</div>
            </Infinite>
        )
    }
}

[developit]

Thanks for the report, I'll take a look! Likely related to refs or findDOMNode I think.

[developit]

@slmgc I believe this should be fixed in the latest preact (5.6.0) & preact-compat (2.2.0). Let me know if I'm wrong!

[slmgc]

@developit thanks! I'll test it today.

[slmgc]

@developit sadly, this issue is still reproducible :/

[developit]

Ah, darn. I'll do a better job of reproducing locally, sorry!

[slmgc]

@developit do you need any help with repro?

[developit]

I should be alright. I really do wish one of the various bin sites (webpackbin, esnextb.in, jsbin, etc) would add support for module import aliasing. I've got an open issue on webpackbin for it. That would make these library integration repros nice and easy!

[mikekidder]

@developit can replicate issue whenever the Infinite component properties elementHeight exceeds containerHeight for all initially loaded children. So something is causing the component to disappear when scroll bar get activated during the initial load.

Played with another sample in Preact (works fine) which dynamically loaded 1000+ elements in 2sec intervals, but the initially loaded data did not exceed the containerHeight. Hope that makes sense. Can provide sample if needed.

@slmgc Side note - should be loading styles like:
<div style={{height: elementHeight}}>content1</div>

Changed variable name for illustration purposes

[developit]

Do either of you happen to have a screenshot or log (w/ stack trace) of the error that gets thrown? Thanks

[mikekidder]

That's the thing really, not really one... Take note that the Closure for the component is completely missing in action. rather that this is referencing an object not cl

[developit]

If anyone still has a repro for this bug, I've just published preact@6.0.0 under a beta dist-tag (npm i preact@beta), and there was a corresponding update to preact-compat yesterday that fixed a possible case where String refs would be left untransformed and thus never invoked. I'm hoping the combination of those two updates fixes this issue!

[slmgc]

@developit great, I'll check it!

[slmgc]

@developit sorry for the delay. Nope, still fails with the same error :(

[developit]

K, I'll keep digging haha

[jugimaster]

Does this indicate a problem with Preact though, or React-infinite? .. or perhaps how either one is used?

[slmgc]

@jugimaster I think it's a minor compatibility issue with preact, because this library works fine with React.

[jugimaster]

@slmgc Yeah, but there are lots and lots of libraries that were built to work with React, in more or less wacky ways.

I don't see why Preact would need to adapt itself to all of them, especially considering a lot of them are bound to be kind of hacky.

Preact's scope is narrower than React's anyway. I think it would benefit from limiting itself to being just another way to have Virtual DOM + Components, without worrying about React's massive ecosystem.

[slmgc]

@jugimaster I don't think it's a good idea limiting preact to being just another virtual DOM implementation. Preact rocks because you can switch from React to preact without any changes to the code. So, actually, it's a good idea to nail down all/most compatibility issues between React/preact/Inferno, etc. So anyone could reuse 99% of 3rd-party components. And I'm not sure to which whacky ways are you referring to? Is changing props, using life-cycle methods and manipulating DOM-elements considered hacky? I don't think so.

[jugimaster]

@slmgc

Well, if Preact does exactly what it's supposed to, according to React's public API, wouldn't any library that only relies on the public API work too?

Without looking into it, I'm guessing that in that case, if a library doesn't work with Preact, it relies on something that happens "behind" React's API, or some sort of implementation details.

Maybe React does something in a certain order, or at a certain time, and somehow a library depends on that behaviour? -That's something Preact shouldn't concern itself with, especially when most tools/libs in the front-end world are solutions in search of problems that we'd all be better off not using anyway.

[slmgc]

Well, if Preact does exactly what it's supposed to, according to React's public API, wouldn't any library that only relies on the public API work too?

@jugimaster No, the thing is, there are still compatibility issues left, that's why some libraries don't work as expected with preact. As an example, while working on react-hint and compatibility with Inferno, I with Dominic Gannaway (creator of Inferno) were able to find and resolve an issue with Inferno's shouldComponentUpdate lifecycle method.

[jugimaster]

No, the thing is, there are still compatibility issues left, that's why some libraries don't work as expected with preact

Well, that's what I'm talking about. From my point of view, as long as Preact implements React's public API correctly, it has no "compatibility issues" at all.

Libraries written against React (and its implementation details) may have compatibility issues with Preact :)

Again, most tools/libs in the JavaScript ecosystem shouldn't be used at all. As a random example I saw today, why does preact-redux exist?

(especially if Preact's author is doing his darndest to adapt Preact to any and all React libs)

[slmgc]

Well, that's what I'm talking about. From my point of view, as long as Preact implements React's public API correctly, it has no "compatibility issues" at all.

@jugimaster but it's not, that's why there are some libraries which aren't working with preact. And that's why it's a good idea to solve these compatibility issues. It helps to cover preact with more tests and makes it more stable/compatible.

[jugimaster]

but it's not, that's why there are some libraries which aren't working with preact

I doubt it :) But maybe I'm wrong!

Either way, I hope @developit takes this thread into consideration.

[developit]

Hiya! Preact pretty accurately matches the React API, but it's possible there are implementation details from React that libraries then depend on. As an example, the timing of lifecycle events within the diff - a very subtle detail not covered in the React docs, but heavily relied on by libraries.

I'm always happy to try to get to the bottom of compatibility issues, because doing so provides an opportunity to find bugs - libraries that might use React in slightly strange ways have a tendency to point them out more than idiomatic React-style code.

That being said, we've been pretty good about trying to offload some of the crazier compatibility stuff into preact-compat, keeping Preact itself as light as possible.

It's funny you mention preact-redux though - that's actually why it exists. react-redux is well-written but relies on some things Preact intentionally leaves out of its core - namely PropTypes and the silly Children interface. While it's perfectly fine to pull in preact-compat and use react-redux unmodified, for those using preact without -compat, it's somewhat annoying to pay that cost just for Children and PropTypes, knowing their value to you is limited. Supporting both approaches is easy though - you can see just how little was needed to get react-redux working on top of preact here: https://github.com/developit/preact-redux/blob/master/src/compat.js

TL;DR: weird uses help find edge cases, but fixes won't bloat. It's just a debugging exercise 😊

[jugimaster]

Preact pretty accurately matches the React API, but it's possible there are implementation details from React that libraries then depend on. As an example, the timing of lifecycle events within the diff - a very subtle detail not covered in the React docs, but heavily relied on by libraries.

As I suspected! It just seems like a battle you can't win. Why would a lib rely on something like that, besides its authors not being into writing robust software?

I didn't realize preact-redux was your lib. My first reaction to seeing it was something like "oh no, it's starting again.. !" :P

(.. "and soon we'll have stuff like preact-redux-router and of course its lighter, less hacky cousin preact-redux-simple-router, and so on")

[developit]

It's definitely a cat-and-mouse game. Personally I just picked a few really high value (common) libs that were small enough to make depending on preact-compat a nuisance, and rolled them up in order to provide a reasonably quick out-of-the-box experience for preact. I don't think we want to go around forking things just to add aliases to the build jobs though - that's the job of preact-compat, and I think it accomplishes it reasonably effectively.

I do think it's worth making sure preact-compat represents as perfect of a drop-in React replacement as possible, though. For a lot of people, that's a really important feature of Preact. I always write my own code against Preact (since I'm rather biased), but I have definitely found it useful to be able to pull in arbitrary third-party dependencies from the React ecosystem. Sometimes you need to accomplish tasks that are mostly unrelated to the value of the project you're working on, which makes using an off-the-shelf solution quite compelling.

[jugimaster]

Well, at least you're off to a good start. Thanks for your time! :)

[developit]

Turns out this was actually ridiculously easy - createClass binds methods you pass to it, but it was binding them after invoking the constructor, which was where react-infinite grabs its reference to the event handler in question - meaning it had a reference to the unbound function instead of the bound one. Simple fix was just to bind earlier. Your repro is now working for me locally @slmgc ❤️

The fix is released as preact-compat@3.9.2.

[slmgc]

@developit looks like we almost there :)
react-infinite works just fine, but once in a while it fails with this error:

It happens when the new data is loaded and appended to the existing one, like once in 10-15 times or so.

[developit]

@slmgc which version of preact? If possible, try with 7.x (npm i preact@beta)

[slmgc]

@developit preact@6.4.0. Checked with 7.0.1, works fine. I saw this error once more while randomly switching pages, but I wasn't able to reproduce it. I think react-infinite is ok 👍

[developit]

yay! 7.0 will become mainline shortly, it's just in beta as the group of (technically) breaking changes gets solidified.