Material-ui: Pre-V1 Compatibility with React Fiber

[lostpebble]

Problem description

I have tried to update my React version to the new releases of Fiber but it appears that react-tap-event-plugin is no longer compatible with those versions of React.

I make use of Material-UI components such as DatePicker and SelectField, which are not yet on the release map for V1... and there is no real timeline on their release, which is a concern because it would mean that the only thing holding me back from using React Fiber is my reliance on those Material-UI components.

Is there no way to allow the pre-V1 release of Material-UI to forgo the reliance on react-tap-event-plugin (just make use of the regular onClick event listeners) and in doing so allow its use with the React Fiber.

Because of the unknown timeline of when all the gaps will be filled with the newer version of Material-UI, this would require me to probably use other component libraries just to fill in those gaps until Material-UI catches up. If there was an easy way to make the older versions compatible (maybe a global switch which removes the reliance on react-tap-event-plugin), that would be great.

Versions

• Material-UI: 0.18.7
• React: 16.0.0-beta.2

[oliviertassinari]

Thanks for the feedback, maybe we could wait for react-tap-event-plugin to catch up?
Also, I think that we can accept a PR removing react-tap-event-plugin for the onClick callback.

[lostpebble]

@oliviertassinari When I tried to get my project up and running with React Fiber, I saw react-tap-event-plugin was the cause of the problem - so I forked it and tried to get it working with the new flat package of react-dom but after scouring the code for some time and trying to make it work I just couldn't find a way to get it linked up again... I have a feeling that the React team is not going to expose similar endpoints that the react-tap-event-plugin relies on, as _some_ of the things that it used were behind a variable called __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED...

Admittedly I'm not very familiar with the internals of React DOM, but it just seemed like the things react-tap-event-plugin was relying on were no where to be found within the code. Perhaps the guys behind react-tap-event-plugin will know better how to link to the new API - but it seems they're a little slow on responding to the same issues being raised on that side. (https://github.com/zilverline/react-tap-event-plugin/issues/102)

Cool, when I find some time I might be able to take a look at removing react-tap-event-plugin from Material-UI pre-V1, though I also know nothing about the internals of this library either so might not be easy. Is there anyone who perhaps knows more about react-tap-event-plugin and why it was required in the first place? And how one might revert this library's dependence on it back to the "native" onClick instead? I'd be happy to help out, but a bit of guidance would go a long way.

[oliviertassinari]

Is there anyone who perhaps knows more about react-tap-event-plugin and why it was required in the first place?

react-tap-event-plugin was added in the first place in order to skip the mobile delay on the onClick event. Nowadays, this delay has been removed by all the major mobile browsers. So as far as I know, the migration is simply a matter of replacing onTouchTap with onClick.

[lostpebble]

Alright I'll try get on that a bit tonight and scope out the amount of work needed. If it's really just renaming the prop, can hopefully get it changed over quickly.

[lostpebble]

@oliviertassinari really struggling to get set up with the dev environment alongside my project with symlinks yarn link and npm link... Can't seem to work directly on the source without running into all kinds of babel compile errors etc. how do you usually hack around in the project and check that the changes are working?

[oliviertassinari]

@lostpebble Nowadays, I'm running the documentation. In the past, I was using the master branch in one of my projects. I had to add some extra babel plugin in my config, I can't remember. It was a long time ago.

[lostpebble]

@oliviertassinari Okay, I've managed to remove the reliance on react-tap-event-plugin but it appears that React Fiber is now removing support for string refs, at least from what I can tell from the error messages I'm receiving.

I'm going to see if I can make that change along with this one. Should I first make the pull request removing react-tap-event-plugin and create a separate one for changing the way refs are handled?

[oliviertassinari]

Okay, I've managed to remove the reliance on react-tap-event-plugin

Good news! Yes, that's already a major change. Let's make small PRs.

it appears that React Fiber is now removing support for string refs

Oh boy, that's some real determination you have here!

[lostpebble]

@oliviertassinari yea... just realising now the scope of changing the refs over... 😅 Will have to see if I can devise some kind of fool-proof regex / process to handle it. Might take some time.

Okay, I'll make the pull request now then for the react-tap-event-plugin changes.

EDIT: React v16 might support refs after all... I'll check tomorrow. Using yarn / npm link and developing stuff like that brings a myriad of other problems - like react being included twice in some cases. The error mentioned the string refs but also mentioned React potentially being included twice which might have been the case since I had just updated to v16 to test out compatibility.

[lostpebble]

Alright can confirm that the changes are working for both the new React Fiber and v15 in my project, and all the tests are passing.

I'm just getting some strange behaviour in React Fiber related to DropDownMenu's not popping up in the correct place (using a fixed position in the top left - or however you've set the anchor probably - of the screen instead of anchored to the button element which triggered it).

Since this pull request works fine in the v15 React, this could be a regression (or stricter progression) of some kind in the new React code base. I'm going to be looking into that now.

[lostpebble]

I've managed to fix the [PopOver] problem now in another branch. It appears that all my commits from this pull request are getting bundled into that one now though... So not sure what to do. Might have to wait for the first one to be accepted? Or maybe I can somehow remove the previous commits from that branch...

[salterok]

Would this be landed on 0.18.x?
I'm currently have admin panel made on 0.18.x version and can't move to v1 branch nor to react@16.

[oliviertassinari]

@salterok We will release it as v0.19.0.

[oliviertassinari]

@lostpebble What's missing for closing this issue? Regarding the timing of the release, I think that we should aim for the first pre-release of React so we are not too early nor too late.

[lostpebble]

@oliviertassinari as far as I can tell, this issue is actually resolved now. I'm currently using master branch in my projects with React v16 beta and haven't had any problems.

Waiting for the first RC sounds reasonable, just to make sure everything is actually up to scratch - although I do think nothing much is going to change now going into the full release. So up to you whenever you see fit.

Guess I'll close this one then 🙂

[oliviertassinari]

Awesome, it's also making the master branch a step closer to the v1-beta one :).

[oliviertassinari]

Thanks a lot for your effort here!

[lostpebble]

Glad to help out! It's a great library and I've used it for a while, was time to give back a bit :)

[maierson]

This is great work. Thank you. Personally I'd love to have this asap as we're working through the migration to Fiber (which is apparently out to all Facebook by now) and this pesky react-tap-event-plugin on the SelectField is holding us back.

[oliviertassinari]

@maierson The thing is, we also want to encourage people migrating to v1-beta. I will try to release it this weekend.

[maierson]

@oliviertassinari I'm also very interested in that (and would much prefer it). Just waiting for the SelectField and Popover which we use a lot. Thank you again.

[pikzen]

@oliviertassinari The thing is, v1-beta is either incomplete for many people's needs, or said people don't particularly want to rewrite major parts of their app. I happen to fall in both groups. Thanks for attempting to get this release out :)

[oliviertassinari]

We have released 0.19.0. Please report any issues with react@next.

[maierson]

@oliviertassinari @lostpebble Big thanks !

[teknologist]

@oliviertassinari @lostpebble Big thanks indeed!

[pikzen]

Worked flawlessly here, on a medium-sized application. To ease up transition, I just want to inform users that only onTouchTap handlers have become onClick. Other things, such as onActionTouchTap or anything that includes TouchTap in the name has not changed.

[mbifulco]

Echoing @pikzen's comment above - does that mean that react-tap-event-plugin is still required for onActionTouchTap events?

[pikzen]

It is not, they just have not been renamed. From what I can see, they all internally call onClick.

[mjclawar]

When react-tap-event-plugin is not used with [email protected], there appear to be lingering issues:

1. DropDownMenu does not open the menu on click
2. Dialog with modal={false} does not close via click on the background (close buttons work).

No issues when react-tap-event-plugin is used.

[mbifulco]

I found the same @mjclawar - since this is closed, that should probably be logged as a separate issue (or this should be re-opened).

Release notes make it seem like react-tap-event-plugin is no longer a dependency.

[mjclawar]

@mbifulco just opened at #7829. Have you found any other components with "touch-plugin-required" events?

[mbifulco]

So far I have not - I use a fair spread of the material-ui controls throughout my project. I'll report back there if I find any more. Thanks!