TNRIS / dataHub

ReactJS frontend web application for browsing, downloading, and inquiring about TNRIS public data and historical imagery.
https://data.tnris.org
7 stars 0 forks source link

Update @material packages #271

Open JasonKleinert opened 4 years ago

JasonKleinert commented 4 years ago

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 commented 3 years ago

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

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 commented 3 years ago

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

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 commented 3 years ago

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 commented 3 years ago

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

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 commented 3 years ago

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 commented 3 years ago

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.