Skip to content
This repository was archived by the owner on Jan 14, 2025. It is now read-only.

I figured out how to reuse the component class implementations from MDC directly in React #4

Closed
jamesmfriedman opened this issue Mar 10, 2018 · 11 comments

Comments

@jamesmfriedman
Copy link

@lynnjepsen, I know I've been blowing you up on discord a bit, but I had to share this.

This is currently hacked together, but the fact that it works is crazy. I found a way to import the actual component class from MDC web and directly use it in React.

  • I import the class
  • Pass it to an HOC
  • I create a new React Component
  • I copy the MWC component class descriptors over to the new class
  • I handle the DOM refs in a way that reflects the way the MDC component expects them
  • The only things I need to specify at this point are things that work a bit different in react like setting classes and a syncWithProps function which can handle the react render cycle.
  • It is currently hacky as hell, but I thought about this an hour ago and I'm still in shock it actually works.

Check it out

  • Don't mind all the code in the commit just look at base/MDCFoundation and IconToggle/index.
  • npm run storybook, navigate to IconToggle

rmwc/rmwc@17cdee6

What does this mean?
I don't have to reimplement a single foundation adapter! I can use all of the ones already written in MWC web with minimal tweaks. I need to work through this more to find the edge cases and do a real implementation, but I'm pretty stoked right now.

@jamesmfriedman
Copy link
Author

jamesmfriedman commented Mar 15, 2018

Update:

I've managed to get Ripples, IconToggles, and Sliders implemented using this strategy and I'm happy to report it works splendidly. The Slider was a good one to work through because it is a relatively complex component.

Some other things to note about this implementation

  • I am directly reusing the adapter and classes implemented in the current material-components-web
  • I can selectively override any of the adapter methods I want if there is a more appropriate react way to do it
  • I changed the emit function in the base class to dynamically emit events through props. MDCSlider:input -> props.onInput
  • syncWithProps is the magic function that gets run, and really all that needs to be done for the most part is diffing the prop value with the built in getters that MWC already has (nextProps.value !== this.value) and then responding accordingly.

I have no clue if this strategy interests anyone, but you get a ridiculous amount code reuse and consistency with MWC.

@moog16
Copy link

moog16 commented May 30, 2018

I did some investigation and tests to see if there were any issues with your text field and button component. It all works as expected. I think your way of doing things is totally valid, and is easier to maintain as MDC Web outputs new features/code. I am interested in performance of MDC React vs RMWC. Although MDC Web is a very performant library, so I think if we find differences they would be negligible.

@jamesmfriedman
Copy link
Author

That’s awesome to hear Matt! Would love to consolidate efforts in any way possible. I have a few non foundation components still but I plan on finishing up the transition with the next release.

@mrchief
Copy link

mrchief commented Jun 3, 2018

If this approach works without any significant performance overheads, then it'd enable fast feature parity with MDC Web and React MDC.

With all due respect, the current list of ready components on the readme is not enough for anyone to start using this in a real world app.

@rjdestigter
Copy link

Does this implementation though prevent props from dictating component state? (React props driving what tab is selected, or if the dialog is open for example.)

@jamesmfriedman
Copy link
Author

Nope. There is a function that gets run on reacts update cycle that also gets run when mdcs internal state changes. This ensures they sync bi-directionally. There might be an edge case hiding somewhere but I haven’t found it yet.

@rjdestigter
Copy link

rjdestigter commented Jun 5, 2018

@jamesmfriedman That's awesome, because I'm really liking rmwc. Here's an example of an edge case maybe? https://codesandbox.io/s/1z5roozm17 Unless the Tabs components haven't been ported over to use the foundation/adapter implementation yet?

Never mind: rmwc/rmwc#189

@jamesmfriedman
Copy link
Author

Literally ported them over this morning! They’re getting released later this week with MDC 36. With this last push, the foundation integration is totally complete.

@moog16
Copy link

moog16 commented Jun 5, 2018

@mrchief totally agree. We do not have all the components we would like to have currently. I can say that as we build MDC Web components to the new spec, we are planning to build the matching MDC React components.

@lynnmercier
Copy link
Contributor

We've decided to stick with the Foundation/Adapter approach. While this is more time consuming, we think using React's synthetic event system is inline with React development. We worry that using DOM events is anti-pattern in React, and will cause unintended consequences.

@jamesmfriedman
Copy link
Author

@lynnjepsen

Understood on wanting to take the long way around. Just a heads up for you though:

  • Yes, React uses Synthetic events for the VDOM. This is fine for things like clicks, mouse events, etc. In all sensible places in RMWC, I use them.
  • When it comes to Custom events that are more abstract and don't necessarily have an interaction associated with them, I'm not aware of a way to represent those as SyntheticEvents. One example I can think of is Dialog:cancel, Menu:cancel, etc.
  • Even if you could surface the SyntheticEvent constructor, you'd still have to find a way to trigger it.
  • The closest thing I've found to this is Enzymes simulate api for events. You might start there.

Happy hunting
https://www.google.com/search?q=React+manually+fire+syntethic+event&rlz=1C5CHFA_enUS761US761&oq=React+manually+fire+syntethic+event&aqs=chrome..69i57.5905j0j7&sourceid=chrome&ie=UTF-8

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants