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

The limitation with react-tap-event-plugin as a dependency. #6587

Closed
SukantGujar opened this issue Apr 12, 2017 · 4 comments
Closed

The limitation with react-tap-event-plugin as a dependency. #6587

SukantGujar opened this issue Apr 12, 2017 · 4 comments
Labels
bug 🐛 Something doesn't work

Comments

@SukantGujar
Copy link

Please see my original issue posted here - zilverline/react-tap-event-plugin#98.

We currently have a rich Application Bar component which is shared across several React and non-React applications. The component is bundled into two flavors -

  1. As an importable React component.
  2. Wrapped in a script shell which is included in web applications through a traditional script tag. This allows the component to be hosted in non-react web applications as well. The wrapped version internally calls ReactDOM.render to mount the component on a host provided dom element. The webpack bundling configuration of this script has React and ReactDOM declared as external dependencies to ensure we don't enrage React if it's already loaded in the host website for any reason.

Currently this component uses React-Bootstrap components. Recently we started migrating to material design and having past success and familiarity with material-ui, it was our first choice. However in this case, we soon got blocked by the react-tap-event-plugin dependency. This plugin currently doesn't seem to have a way to work with externalized React dependencies and this causes the plugin to not register currently with ReactDOM in the global context (window.ReactDOM). This is a complete blocker for us. And looks like despite the browsers now inherently supporting the features polyfilled by this plugin, we still get loads of errors with material-ui as it internally adds onTouchTap props on its components, causing React to throw up errors for unsupported propTypes.

Is there a possibility of a workaround here? We are willing to pull a PR if you folks can guide us through the possible solution. Alternatively, do you already have this on the road-map and would it be possible to share estimates (very rough will do)?

Thanks.

@mbrookes
Copy link
Member

it internally adds onTouchTap props on its components, causing React to throw up errors for unsupported propTypes

If material-ui is passing internal Material-UI non-dom props to dom element, please open an issue with specific details (and by all means submit a PR).

Alternatively, do you already have this on the road-map

react-tap-event-plugin is removed in the next branch (npm i material-ui@next)

@SukantGujar
Copy link
Author

We noticed it with components which render a <button> tag, e.g. RaisedButton etc. Anyways, will check out the @next version. Thanks!

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 14, 2017

I think that we should close the issue. I'm not sure it's deeply related to Material-UI nor we want to invest into it

@SukantGujar
Copy link
Author

sure, thanks folks!

@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Apr 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work
Projects
None yet
Development

No branches or pull requests

3 participants