Need help/architecture review on React props

react
1

Hi all,
So, after months of research and prep work, I bit the bullet and rewrote a lot of the Clinical Meteor boilerplate into React. Generally speaking, I’m really happy with everything, and it’s working great. The initial UI renderings were basically everything I had hoped they would be, and React is proving to be the right decision.

However. We had a number of constraints which prevented us from using the typical tutorials and walkthroughs. Constraints including Material UI design, opacity rendering requirements, an interest in avoiding mixins, and wanting to leapfrog to using extends React.Component syntax. So, we sort of started with the libraries that we really wanted to use for our user interface (React-Toolbox, SCSS, etc) and worked backwards.

Everything is working really great… except for props. We’re basically just using Session variables, and bypassing the props cascade entirely. Using this.data instead of this.props. That’s worked for the past few months; but it’s come with costs and technical debt. We’ve got a console log full of warnings and error messages, which I’m not entirely sure how to debug.

1336×1452 246 KB

The app works surprisingly well despite all these warnings. However, as you might imagine, there’s noticable refresh lag. Recently, things have deteriorated to the point that clicking on the account signin/signout button will cause the entire app to pause for a few seconds and refresh. :confused:

So… it’s time to pay down the technical debt. And frankly, on this one, I could use some help. If there are any React wizards out there in the audience, I could really use some help in sorting out what’s going on with these props and figure out an architecture strategy for the React classes in my app. I’m hoping for a half-day or full-day working session maybe?

I sort of need for this graduate school and a pending client project; so I can compensate hours worked (payment through venmo?); and the code will remain open-source.

Source code to the app is at GitHub - clinical-meteor/meteor-on-fhir: Meteor using Fast Healthcare Interoperability Resources (FHIR)

Halp?

3

the error logs tell you exactly, what’s wrong:

it’s not allowed to pass properties to html-elements, that these elements do not support. (e.g. “active” or “pullRight”, or whatever you have there).

This is since one of the newer react version, sometimes some older version of ui-libraries in react may also show these warnings.

4

Yes, they tell me exactly what’s wrong. What they don’t tell me is how to fix it.

And to be perfectly honest, given these error messages, I don’t even know which components are throwing the errors. Or how to access them to fix them. They’re pulled in through NPM, so how the hell am I suppose to remove the props?

I don’t even know where to begin with debugging this… :frowning:

5

There has been a lot of work in the Material UI issue tracker where these are being removed as on ongoing project. For example this, and more generally.

For what it is worth, we are not using this.data or session variables. Use mobX quite heavily to manage everything state. So the top level component (recipient of props from container) has an observable store, and a function to make changes to the store. Lower components depending on complexity have an observable UI store.

Currently just passing the changeStore function down on props, but the smart way to do this is to turn the store into a class and then add @computed functions which can be exercised by everything downstream.

Testing is limited to changing about the store and doing shallow renders, but do more testing on the methods and helper functions per collection.

Not sure if above is useful, but can post simple examples if needed. The testing around both isClient and isServer sucks at the moment though, so integration and full app tests are shite. Product is still in alpha, so all good. :slight_smile:

2 Likes
6

@tathagatbanerjee - that actually did help. I was having a bad day yesterday, and things seemed a bit hopeless. And just knowing that other people were working on it; learning that the warnings aren’t necessarily my code, per se; that the package authors have mostly pushed updates; knowing where to even look on GitHub… all of that helped.

All that lead me to… ehem; rebooting my workstation. And it turns out some of the errors I was having went away when I pulled a copy into a clean directory. Apparently, some of my NPM caches had gotten out of sync or something.

This seems like a much less daunting issue to deal with today. :slight_smile:

2 Likes
7

I am also facing the Unknow props problems.
By looking at your code, i imagine that we have a base in common which might be base itself from themeteorchef.
To remove those warnings, i use ^((?!Unknown props).)+$ as regex on chrome tools so that i can focus only on my errors.
Hope it helps,

2 Likes
8

This may also be helpful. Not sure, but it showed up in my github tracker today.

1 Like