Incompatibility with Preact

[dandv]

Description

Can't get react-md to work with Preact, even with preact-compat installed. I've created an app skeleton with the "default" template, followed the react-md installation instructions and additionally loaded webfontloader (should this be mentioned on the Install page, BTW?), but I get the error below when I npm run dev:

Images/Screenshots

Link to a gist or code sample where the issue can be reproduced

https://github.com/dandv/preact-react-md/

I've also filed the issue in Preact's repo at preactjs/preact-compat#502.

Version

Preact 8.3.1
React-MD 1.5.0

[mlaursen]

Hmm, I haven't worked with preact before, but this might be because I use ReactDOM.findDOMNode to set the containers which looks like returns null instead of the element in preact.

[dandv]

Turns out findDOMNode returning null had recently been fixed in preact-compat 3.18.3, yet my reproduction repo still shows the issue when using 3.8.14. Any other ideas?

[billneff79]

I believe the issue is with preact itself, not with preact-compat or its findDOMNode implementation. The problem arises in that react-md's InkContainer component is a High Order Component (e.g. a proxy to another component) component with no real html root node itself (it just turns around and renders another component TransitionGroup:

react-md/src/js/Inks/InkContainer.js

Lines 411 to 416 in 6297690

	<TransitionGroup
	component="div"
	style={style}
	className={cn('md-ink-container', className)}
	ref={this._setContainers}
	>

For some reason that I am unaware of, preact calls the ref callback function before rendering the Higher Order Component, and thus base on the component rendered by a Higher Order Component is not set immediately when the ref callback is called

https://github.com/developit/preact/blob/8aa7ec9e87e34596d0ec292d12fb9f79135e382f/src/vdom/component.js#L127-L143

https://github.com/developit/preact/blob/62c04e0e373c24e0e2b329457b85f233753b2aa3/src/vdom/component.js#L46-L55

In this scenario, the renderMode is NO_RENDER for a Higher Order Component because it's top-level child is a function/component, not a regular DOM node.

What this all means is that HOCs in preact need to put a delay around their children's refs callbacks to make sure this.base will resolve. Here is a simple reproduction jsfiddle (look at the console output):

http://jsfiddle.net/billneff79/es9xuL65/

I don't know if this is a bug or a feature in preact, but I know you could easily work around it in react-md by modifying the ref callback in InkContainer here:

react-md/src/js/Inks/InkContainer.js

Line 180 in 6297690

_setContainers = (group) => {

with a setTimeout wrapper, like this:

_setContainers = (group) => { 
    if (group !== null) {
      setTimeout(() => {
        this._inkContainer = findDOMNode(group);
        this._container = this._inkContainer.parentElement;

        if (this._container) {
          this._initOrRemoveEvents(this.props);
        }
      });
    }
}

[marvinhagemeister]

Disclaimer: I'm part of the preact team

@billneff79 That is an issue with how refs are applied in current preact. Our issue to track that is preactjs/preact#1177 .

[mlaursen]

Thanks for looking into this!

I'm not entirely sure if I want to apply a patch to fix this just for preact since I have mainly been focusing on just React itself and wanting to keep the scope low since I'm the only main developer. If this is an extreme blocker and can't wait until I finish the v2 branch, I can try adding the suggested setTimeout fix above. Otherwise I think I would like to hold off "fixing" this until the v2 branch where I stopped doing HOCs (and some hacky DOM node finding) for Inks and Tooltips.

[dandv]

If this is an extreme blocker and can't wait until I finish the v2 branch

This is as extreme as a blocker can be, since it means react-md can't be used with Preact at all :) Is there a timeline for the v2 branch?

[mlaursen]

Sorry, I actually think it's unreasonable for me to say wait for v2 branch now that I think about it.. It's probably a good few months away :( (first pass should be near the end of this month though).

So I'll look into doing the suggested fix above and your repo to see if that fixes it.

[mlaursen]

I haven't seen any problems by adding the setTimeout workaround for my existing react apps as well as the preact demo, so I'll release a patch with this change tonight

[mlaursen]

There ended up being some additional conflicts that I didn't find in my testing so I reverted the workaround until it can be resolved.