Update @material packages

[JasonKleinert]

Material design packages have moved to v6. Starting in v5 they began using the sass module system. Sass-loader is experiencing bugs loading modules that use both @import and @use. There are some potential workarounds that require editing webpack configs. We can not edit these unless we eject our create react app, which would not be preferred.

Sass and sass-loader are locked in a discussion to resolve the above mentioned issues, documented in this github issue. I'd suggest we hold off on upgrading material from v4 > v6 until they can work out their differences and see if things work a little smoother after an update to sass-loader is released.

ADDITIONALLY, we are using node-sass to compile our sass files. Node-sass does not currently support sass modules, so we will need to use dart-sass instead when we begin the upgrade process. Unless node-sass is updated as well.

Yay javascript!

[ctrepka]

Solution

Decided today that a slow migration to material-ui library from material-web-components is likely the best path forward. Will migrate components to use material-ui incrementally.

Rationale

• breaking changes in the material-web-components repo which are not compatible with node-sass, the typical sass implementation for create-react-apps.
• Ejection would be required, in this case, requiring more complexity and management of configuration than desired.
• The issue has been known by both the developers of sass and by the material-web-components library for some time now without much substantive action.
• Material-UI, the react-focused implementation of MDC, which is backed by a handful of paid sponsors and a lively dev community, seems to be the most stable path forward without ejecting from Create-React-App.
• This solution allows us to test material-ui as it is implemented, and to address other issues as they arise if more pressing than this migration.

If the solution proves to be more difficult than anticipated or more complex than ejecting as a long-term solution, we can always revisit this solution.

[ctrepka]

Posting this from my last commit as a status update on migration efforts:

Made some progress on MaterialUI ToolDrawer today. Primary concerns noticed today include

• migrating from class components to functional components; MaterialUI is almost completely functional and plays better with functional components.
• By extension, the functional components also use Hooks rather than the old class-based life cycles.
• Finally, the components take advantage of functional, hook-based cssInJS styling, which will need to factor in to this migration plan as well.

So, key take aways are that this migration plan can/should(?) include migrating from class-based architecture to functional architecture for most components. Will start with stateless components as those are very easy to migrate. (See ToolDrawer.jsx in my latest commit, for example, and compare to Catalog.jsx where I have maintained the class-based component architecture)

[ctrepka]

I think I'm at a good place to hop off the MaterialUI changes for a bit. I'll be submitting a PR this afternoon. Basically, the changes encompass almost all of the Catalog View. Some scss -> cssInJs still needs to occur, and the next chunk I plan to work on is the collectionFilterMapView and the other geofilter-related components. Meanwhile, I might tackle #267 while I'm already reworking the code.

After that, I'm pretty confident the rest of the app should follow quickly.

[ctrepka]

Okay, so this might be a controversial idea so bear with me...

What if we just rewrote DataHub entirely? We maintain the look & feel, the UI, etc with only minor changes which are already GitHub Issues in this repo.

Here's my rationale

• Much of the codebase needs to be rewritten per my comments above
• Jason and I came to the conclusion that, due to the fact that this material-ui transition is relatively low-severity maintenance work, we probably don't need to roll out gradual UI changes. A gradual roll-out really just adds unnecessary complexity to the process.
• That means we can rewrite the app in a separate branch/version and make gradual commits to it over time while making only necessary updates to the current, live DataHub app
• React and Redux both are moving toward the functional/hook-based convention. Material-UI already is, and doesn't play well with class based components. Moving the entire codebase toward these conventions is advised.
• As Jason and I noted last week, Redux Hooks provide the ability to remove a lot of unnecessary complexity from the codebase, including the component containers.
• Given the way the scope of this issue has escalated, it might be a good indication that the codebase needs a solid update/overhaul. The old code isn't bad, but React conventions/paradigms have changes a lot in the past few years.
• In my opinion, starting from the ground up and reusing old code where necessary might be a better approach than iterating through each file in the repo trying to translate one paradigm to another. A lot of the time, there isn't a 1:1 translation.
• Finally, I believe I had discussed with Adam and Jason during the changes to the navigation bar on DataHub Improve user access to Shopping Cart once an item has been ordered from collection view (UX) #280 that a rewrite may be necessary at some point. I think this might be a good time to hop on that, and I'd be happy to tackle this challenge

Let me know what y'all think. I know this is a big proposal, and I know this is a really big undertaking with relatively little reward; but given that React seems to be stabilizing in functional/hook land as of version 17 (which basically has no new features), it might be a good move for the overall stability of the repo.

[JasonKleinert]

My opinion is that if we are going to rewrite the app we should also consider a redesign as well. It would be beneficial if we could develop design and functional requirements before we begin to rewrite the codebase. This should make the development a lot easier and more straightforward. I would also like to suggest that we develop tests while we do the rewrite. We could consider a test driven approach or not, but having good test coverage will make this app more stable and maintainable going forward.

Was this discussed at the scrum on 11/10/2020 while I was out? It would be nice if we could have a more formal meeting aimed at developing a strategy for the design, functionality, and development.

[ctrepka]

Hey @JasonKleinert , we did discuss this a bit on 11/10/20. I think the Material updates, paired with the updates to React, constitute a big enough change to the codebase that it makes sense to do a rewrite. Can't speak for everybody else, but I think we were basically all in agreement on Monday that a rewrite is a good path forward. I like your idea about having a more formal planning meeting, I think I would personally benefit from that and it would help us get on the same page regarding what features we can implement and how to set ourselves up to grow into new ones, including the improvements we piece out of the survey results.