ProjectSidewalk / SidewalkWebpage

Project Sidewalk web page
http://projectsidewalk.org
MIT License
84 stars 24 forks source link

Update versions of languages/libraries/frameworks #1258

Open misaugstad opened 6 years ago

misaugstad commented 6 years ago

I made an issue for one specific instance of this (#1092 -- updating turf JS library), which would be particularly useful to have updated. But it may be a good idea to update the versions of languages, libraries, and frameworks we are using in an attempt to "future proof" Project Sidewalk. We may not close out this next batch of cities for another 2 years or more, so now would be a really good time to make these transitions. Especially since the project was started more than 2 years ago, and most of the software we are working with are at least one major release out of date.

Updating this software would require

  1. Checking the changelogs for the various bits of software to look for breaking changes that would impact us
  2. Checking for compatibility between the various pieces of software we decide to update, to make sure the versions we try to move to are compatible with the other software we update (or don't)
  3. Figuring out which things need to be updated together, and which things can be updated alone. For example, using a newer version of some Scala library may require a newer version of Scala, so those need to be updated together. But there may be some JavaScript library that can be updated without updating anything else.
  4. Based on the dependencies we discover from step 3, we pick which things we really are going to try to update, and we create issues that are as small as possible (updating a single library if possible, but updating multiple together if we determine them to be interdependent from step 3).
  5. For those that we decided to update, go one issue at a time, update the appropriate software, and test thoroughly, looking at the changelogs for extra guidance in what to look for and test.

Some things we might want to try updating:

misaugstad commented 5 years ago

I've been trying for a couple weeks to upgrade the Play framework from 2.3 to 2.4, which requires updating almost all of the back-end libraries (most notably, Slick). This is a huge project and I need to take a break from it for a bit. But the current state is preserved in the branch 1258-update-play-framework

jonfroehlich commented 5 years ago

What's the current status in general? I know you've been pushing hard on this... how much time do you estimate is remaining?

misaugstad commented 5 years ago

Yes thanks @jonfroehlich good point :grin: So after updating the versions of the relevant libraries in the build.sbt, the bulk of the updates that have to be done are from updating the Slick library from 2.1 to 3.1.

With that update, it modifies the return type of all of the queries to have them wrapped in Futures. This is definitely better, since it gives us a better way to reason about asynchronous computation/queries and because we were wrapping the final result in a Future anyway when sending the results to the front-end.

I modified almost all of the database table model files (*Table.scala) to use the correct return types, but most of the controller files need to be updated to deal with the updated return types correctly. Fixing the rest of these might take a few days.

What I have been stuck on is a problem with the slick-pg library which houses the postgres extensions for Slick that we are using. I haven't gotten any traction with it on stackoverflow, but I have been in contact with the author of the library on Github (though there has been some miscommunication). I am planning to continue working through this problem while talking with the author of that library. Once the issue is fixed, I will probably come back to finish updating our code.

The final thing that needs to be upgraded is the play-silhouette library from 2.0 to 3.0.4. This library deals with authentication, and I know that there were some breaking changes between 2.0 and 3.0.4 where we need to update things. I have not assessed how long that would take. But for a sense of scale, the number of compiler errors from Slick after changing the version number was something like 500 (down to about 175 now), and the number of new compiler errors after I updated the play-silhouette version number was around 20.

jonfroehlich commented 5 years ago

Thanks Mikey. Appreciate the detailed update.

Sent from my iPhone

On Nov 12, 2018, at 5:52 PM, Mikey Saugstad notifications@github.com wrote:

Yes thanks @jonfroehlich good point 😁 So after updating the versions of the relevant libraries in the build.sbt, the bulk of the updates that have to be done are from updating the Slick library from 2.1 to 3.1.

With that update, it modifies the return type of all of the queries to have them wrapped in Futures. This is definitely better, since it gives us a better way to reason about asynchronous computation/queries and because we were wrapping the final result in a Future anyway when sending the results to the front-end.

I modified almost all of the database table model files (*Table.scala) to use the correct return types, but most of the controller files need to be updated to deal with the updated return types correctly. Fixing the rest of these might take a few days.

What I have been stuck on is a problem with the slick-pg library which houses the postgres extensions for Slick that we are using. I haven't gotten any traction with it on stackoverflow, but I have been in contact with the author of the library on Github (though there has been some miscommunication). I am planning to continue working through this problem while talking with the author of that library. Once the issue is fixed, I will probably come back to finish updating our code.

The final thing that needs to be upgraded is the play-silhouette library from 2.0 to 3.0.4. This library deals with authentication, and I know that there were some breaking changes between 2.0 and 3.0.4 where we need to update things. I have not assessed how long that would take. But for a sense of scale, the number of compiler errors from Slick after changing the version number was something like 500 (down to about 175 now), and the number of new compiler errors after I updated the play-silhouette version number was around 20.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

misaugstad commented 5 years ago

Okay when changed to the newer version of the play-silhouette library, we got all those new errors from that upgrade, but I no longer see an error from the slick-pg library that was holding me up. Idk if these new errors are just trumping the old ones or if updating the library fixed some problem with a shared dependency, so I'm going to try and resolve some of the problems from upgrading the play-silhouette library and see if the slick-pg errors come back!

jonfroehlich commented 5 years ago

Nice. Fingers crossed. I so want this all to work out! 🤞

Sent from my iPhone

On Nov 13, 2018, at 11:01 AM, Mikey Saugstad notifications@github.com wrote:

Okay when changed to the newer version of the play-silhouette library, we got all those new errors from that upgrade, but I no longer see an error from the slick-pg library that was holding me up. Idk if these new errors are just trumping the old ones or of updating the library fixed some problem with a shared dependency, so I'm going to try and resolve some of the problems from upgrading the play-silhouette library and see if the slick-pg errors come back!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

misaugstad commented 5 years ago

Okay yep the same bug is back after I fixed most of the new bugs that were introduced when updating the play-silhouette library :) So I am going to take a break from this for a bit again.

jonfroehlich commented 5 years ago

Darn. Is there anyway any of us can help you...?

athersharif commented 5 years ago

@misaugstad updated the libraries but as expected there are breaking changes between these versions, which will be a pain (just tedious work). created a wip pr here for tracking purposes. started with over 2000 errors, down to 1680 🤣

can you review the changes at a higher level? i don't know awful lot about scala so i'd love suggestions on best practices, code conventions, etc 😆. it'd be awesome to get more involved too, if you think we're moving in the right direction.

misaugstad commented 5 years ago

Oh nooo it had been awhile since we last spoke about this so you must have forgotten: I was just asking you to resolve the compiler errors that had to do with play-silhouette authentication library.

In that branch I had already upgraded a bunch of libraries (not all the way to the most recent version, but bumped them at least one large version) and fixed most of the breaking changes. I just hadn't figured out how to fix the breaking changes when I upgraded the play-silhouette library from 2.0 to 3.0.5 and I was just asking you to fix those errors :no_mouth:

athersharif commented 5 years ago

that's where i started lol but ran into issues with the sbt version. they've deprecated the sbt version all the way to 1.x. and then if you update the sbt version then you have to bump the packages up to a version that works with that sbt version. :/

athersharif commented 5 years ago

@misaugstad looked at it a little bit closer. nothing wrong with the versions you're using. sometimes scala goes weird with imports and starts thinking they're relative so when you do import slick.driver.JdbcProfile it starts to think it's a relative package located in models.daos whereas what it really is, is an external package. so if we do import _root_.slick.driver.JdbcProfile instead (in UserDAOImpl), it'd compile successfully 🙂

do we want me to just close out the wip pr or you wanna keep it for future references in case we need to bump to another version?

misaugstad commented 5 years ago

so if we do import root.slick.driver.JdbcProfile instead (in UserDAOImpl), it'd compile successfully

OMG THANK YOU. I forgot I had that issue!! To fix it I just deleted all the compiled/build files (I actually changed the name of the relative package, but it isn't automatically deleted).

do we want me to just close out the wip pr or you wanna keep it for future references in case we need to bump to another version?

mmm you should close it out. we will always have the branch there for reference, but it is cluttered with too many style changes anyway that I don't think it is super useful.

athersharif commented 5 years ago

cool, that's on my fork tho so it might get lost but yeah the styling changes is just unnecessary noise. speaking of, thoughts on scalafmt?

misaugstad commented 5 years ago

Looks pretty good! Want to create an issue for it? I imagine we would have a PR that updates style across the project. We would then add to the dev environment setup instructions! :grin:

athersharif commented 5 years ago

yup, 💯. creating an issue for it, shortly. it will touch a gazillion files so probably should work on it when nothing critical is in a pr state otherwise it'd be a rebasing nightmare. 🤣

athersharif commented 5 years ago

pushed https://github.com/ProjectSidewalk/SidewalkWebpage/pull/1414 as a PR on your branch (also see https://github.com/ProjectSidewalk/SidewalkWebpage/pull/1414#issue-248640129)

misaugstad commented 1 year ago

Just added to the list above: moving away from moment.js for timestamp localization. The project is now in maintenance mode and they recommend moving on to other libraries. Here are some quotes from the moment.js website:

Moment was built for the previous era of the JavaScript ecosystem. The modern web looks much different these days. ... As an example, consider that Moment objects are mutable. This is a common source of complaints about Moment. Changing Moment to be immutable would be a breaking change for every one of the projects that use it. Creating a "Moment v3" that was immutable would be a tremendous undertaking and would make Moment a different library entirely. Since this has already been accomplished in other libraries, we feel that it is more important to retain the mutable API.

Another common argument against using Moment in modern applications is its size. Moment doesn't work well with modern "tree shaking" algorithms, so it tends to increase the size of web application bundles. If one needs internationalization or time zone support, Moment can get quite large.

misaugstad commented 3 days ago

Once we can upgrade past Scala 2.10, I'd love to start using some sort of URI interpolator library to build URIs in a less clunky way: https://sttp.softwaremill.com/en/latest/model/uri.html#uri-interpolator