Skip to content

Conversation

slifty
Copy link
Contributor

@slifty slifty commented Jul 11, 2025

This PR moves the front-end code base to Angular 18. It's part of a larger set of transitions which will ultimately culminate in Angular 20.

I separated the commits into pieces to reflect the "stages" of the move to Angular 18. Once the code is reviewed my plan would be to combine "ng build works" through to the end so there is a single "Angular 18" update which makes all of the needed migration changes in one place.

Related to #590

Copy link

codecov bot commented Jul 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.60%. Comparing base (1a459fc) to head (9ad4df5).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #606      +/-   ##
==========================================
- Coverage   44.65%   44.60%   -0.05%     
==========================================
  Files         369      371       +2     
  Lines       11283    11292       +9     
  Branches     1850     1850              
==========================================
- Hits         5038     5037       -1     
- Misses       6079     6087       +8     
- Partials      166      168       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@slifty slifty force-pushed the 590-update-angular-18 branch 3 times, most recently from 9a03373 to c0a52cd Compare July 17, 2025 18:24
@slifty
Copy link
Contributor Author

slifty commented Jul 17, 2025

@cecilia-donnelly @liam-lloyd @crisnicandrei For this PR what I'm thinking is...

I have the PR itself broken into commits where not every commit stands on its own, but it shows the progress of the migration.

I can then squash appropriately before merge it so there's a single "commit" for "update to Angular 18"

At any rate... I think this is ready for review.

@slifty slifty marked this pull request as ready for review July 17, 2025 18:24
@slifty slifty force-pushed the 590-update-angular-18 branch from c0a52cd to 054794a Compare July 17, 2025 18:50
Copy link
Member

@liam-lloyd liam-lloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be worth including the Prettier corrections in this PR; the migration tool seems to have mangled formatting pretty badly in a number of places. There are a lot of nonsensically formatted arrays, in particular, where it's pretty difficult to find the closing bracket.

@slifty
Copy link
Contributor Author

slifty commented Jul 18, 2025

@liam-lloyd will do... the problem is just running prettier rewrites basically the entire code base because most of the code base doesn't conform to prettier. I guess this is just a matter of spending 30m manually running on each changed file? Or maybe there's a handy command...?

Would you be open to the idea of having my next PR be "run prettier on the entire code base which will fix this PLUS everything else"?

Or alternatively, I could do that pass overall as part of this PR as a final commit?

(ok ok now that I typed all that out: I'll do that first most correct thing)

slifty added 7 commits July 18, 2025 12:22
We want to get to 22 ultimately but we need to move a bit slower
/ update alongside major Angular updates.

This also updates dependabot so that it won't try to bump major versions
of the node types outside of our own node upgrade cycles.

Issue #590 Update to Angular 20
Issue #592 Update development environment to Node 22
@slifty slifty force-pushed the 590-update-angular-18 branch from 054794a to 6f4bedb Compare July 18, 2025 16:37
@slifty slifty requested a review from liam-lloyd July 18, 2025 16:37
@slifty
Copy link
Contributor Author

slifty commented Jul 18, 2025

@liam-lloyd fixed up (separate commit since I'm not entirely sure which introduced the issue, but again I'll be squashing)

@slifty slifty force-pushed the 590-update-angular-18 branch from 6f4bedb to 9ad4df5 Compare July 18, 2025 17:28
@slifty slifty merged commit 46b1226 into main Jul 20, 2025
4 checks passed
@slifty slifty deleted the 590-update-angular-18 branch July 20, 2025 15:47
@slifty
Copy link
Contributor Author

slifty commented Jul 20, 2025

ACK -- normally dev deps are not so comprehensive to need QA but this is a bigger one and I had originally intended to tag @omnignorant for QA before merging, but muscle memory took over just now (this is what I get for checking github on a weekend).

The good news is we have time to do QA retroactively and if there are any issues I'll dive in and fix them in time for deploy.

Tagging @omnignorant now.

@slifty slifty requested a review from omnignorant July 20, 2025 15:50
@slifty
Copy link
Contributor Author

slifty commented Jul 20, 2025

@omnignorant this is an upgrade of the version of the entire frontend framework, so we should do a full test of pretty much anything you can think of.

@cecilia-donnelly
Copy link
Member

@liam-lloyd clicked around a bit when he reviewed and I will do so as well before deploying.

@cecilia-donnelly
Copy link
Member

Changing archives seems to not refresh the file list:

Observed behavior (on dev):

  1. Login to Permanent. You'll see the Private Files of your default archive.
  2. Go to the archive menu (top right) and click another archive. The list of files does not change, though the name of the archive and profile picture do. If you click into another top level folder and then back to Private Files, you'll see the correct items for the new archive.

Expected behavior:
The file list should reload and show the files present in the selected archive right away. (This does seem to be working correctly on prod.)

@slifty
Copy link
Contributor Author

slifty commented Jul 23, 2025

Closing the loop on the above: we aren't able to reproduce this issue at this point, but will be doing another round of tests for the Angular 19 MR

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

Successfully merging this pull request may close these issues.

3 participants