ETT-266: migrate to svelte 5 #118

carylwyatt · 2025-05-30T20:13:06Z

Firebird has been successfully migrated to svelte 5 with two exceptions: the componentsSearchBar and Navbar, which are related via Header.

I wrote a whole novel about my attempts to migrate these components in my notes, but maybe I can sum it up: no matter how far I whittle down this code, VSCode will not let me migrate these two components to svelte 5 or use runes in any way. I keep getting a migration error telling me it cannot set properties of undefined (setting 'next'), but when I add a ton of log statements, everything is defined... and I'm never calling next(). It could be a third-party thing? I like to keep components separated by functionality when possible, but one possible solution is to combine all three of these components into a single Header component if necessary.

While this is mostly ready to go, unfortunately PageTurner can't use this version of the code. I talked with @aelkiss about this a bit, and we have some options:

Migrate PageTurner to svelte 5. This needs to be done eventually, and maybe I should tackle it while this is fresh in my mind.
Pin PageTurner's dependency in package.json to a branch of firebird that hasn't been upgraded (maybe create a new branch called svelte-4 and use that for PageTurner until it can be migrated). The issue with this is that any updates to firebird would either never trickle down to PageTurner or we'd have to udpate the svelte-4 branch with any style/template updates that are applied to the main branch.
A middle ground option: upgrade PageTurner to svelte 5 and migrate the components that reference HT.loginStatus and any other state that trickles down into PageTurner, but leave most of it in svelte 4 syntax for now.

I started working on option 2 on dev-3 and ran into some errors. Since there are some issues, this isn't quite ready for primetime, but a draft PR will do for now.

hathitrust/babel#112 is a draft PR for pinning the firebird dependency in pt to the svelte4 branch of firebird-common.

Testing

Staged on dev-3 (vpn required):

Do some searches, add items to collections, open the feedback modals, log in and out... All of the components are tested in storybook, but maybe some "user flow" testing would be good.

Page Turner is not included in this PR, it will need additional work.

aelkiss · 2025-06-02T16:00:51Z

IMO starting with option 2 followed by option 3 as time permits makes sense to me, but I don't have a sense of how much work option 1 is likely to be.

aelkiss · 2025-06-11T18:17:12Z

src/js/main.svelte.js

+let loginStatusState = $state(emptyLoginStatus);
+
+Object.defineProperty(HT, 'loginStatus', {
+  get() {


I would like to see where else loginStatus is being used just to get a sense of this.

It doesn't look like loginStatus.get or loginStatus.set is being called anywhere (unless somehow that's happening implicitly), so maybe we don't actually need the defineProperty?

In different places, we have loginStatus, HT.loginStatus, and HT.login_status... This is pretty confusing, and I also suspect the usage of HT.login_status in utils.js and CollectionEditModal may not be correct insofar as HT.login_status never seems to get updated except via PingCallbackDecorator which is only being used for storybook?

I went back and looked at my notes about this, and I had to write the get/set methods because in Svelte 5 you can't directly assign $state() to object properties like HT.loginStatus. I could've used a class wrapper, but then I'd have to call .value() or something to get that value, whereas the getter/setter updates the properties of the reactive object so I could leave the code as-is.

The difference between HT.loginStatus and HT.login_status is for handling reactivity. HT.login_status is an object and HT.loginStatus is a store (which is also an object, but with special get/set/subscribe functions). The HT.login_status object is created by ping, I think: https://github.com/hathitrust/babel/blob/2aed2b1fbd567c9817d4973adc504febcbd69a7f/ping/cgi/ping#L131-L148. Then firebird is creating the store HT.loginStatus using that object for reactivity.

Since the postPingCallback (in main.js) is executed on every page, it's gathering that object and updating the store on every pageload. There's a separate but identical postPingCallback function in page turner's main.js that does the same thing.

With Svelte 5, HT.loginStatus is now deeply-reactive state instead of a store. I think we still need these two different properties since they do different things.

loginStatus vs HT.loginStatus is a bit of a mess, but there's a reason for it. HT.loginStatus is a store, but you can't subscribe to just the property loginStatus of that HT object (perhaps HT.$loginStatus ?) without creating a derived store from that store. Using the $: to follow updates to the store is a workaround, and, as mentioned in the svelte 5 announcement post, a footgun. Now that we're using runes, I think this whole thing could be refactored, I just need to think about the best way to handle this state that gets passed around/subscribed to in messy ways.
I think now that HT.loginStatus is using a $state() rune instead of a store, that loginStatusState has "deep state" and I can use HT.loginStatus in place of loginStatus in all the components that use the $: loginStatus = HT.loginStatus trick.
Edit to add: yes, I updated all the loginStatus variables to use HT.loginStatus directly, and I need to test it on dev-3, but the storybook components are working.

Tested on dev-3 and the login logic still works as expected.

While I would love to rename HT.login_status, since it's coming from ping, that's probably more work than is worthwhile.

src/js/lib/cookies.svelte.js

aelkiss · 2025-06-11T18:29:54Z

src/js/lib/analytics.svelte.js

-        _paq.push(['setCookieConsentGiven']);
-      }
-    })
+    if (consent.trackingConsent !== 'true') {


The subscribe method doesn't exist anymore; because this is using reactive state then if there's a change in the value then Svelte will magically cause this method to run.

aelkiss · 2025-06-11T18:32:47Z

src/js/main.svelte.js

+      el.component = mount(apps[slug], {
        target: el,
        props: props,
+        intro: false,


Adding intro: false disables whimsical animations that we did not want.

src/js/designs/ResultsItem/index.svelte

src/js/components/Navbar/index.svelte

src/js/components/AdvancedSearchForm/index.svelte

src/js/components/AlertBanner/BannerMessage.svelte

aelkiss

Reviewed with @moseshll and @carylwyatt. I think there are a just a couple issues regarding the rebase and one component to check with respect to migration.

src/js/decorators/PingCallbackDecorator.svelte

src/js/components/LoginFormModal/InstitutionList.svelte

carylwyatt · 2025-06-16T19:28:42Z

@aelkiss ready for re-review, thanks!

aelkiss

All the comments from the code review were addressed, so I think this looks good!

- use migrate self-closing-tags to fix icon markup - add svelte prettier plugin, add minify false to babel script - convert store to runes - update svelte preprocess - migrate main.js to main.svelte.js - convert writable store to $state() proxy - update index with new file name for vite - update loginStatus logic in login modal components - remove unused components and references to them

carylwyatt · 2025-09-03T13:32:33Z

Updates since the previous code review:

Migrated Navbar and SearchBar to svelte 5 syntax
Rebased against main to get RoleSwitchModal component in this branch
- migrated this component to svelte 5 syntax
- refactored this into smaller/modular components
Removed unused CSS/added :global modifier to silence console warnings

Edit to add: I checked the storybook and realized something weird was happening with the navbar stories. There were some bugs related to state handling in the navbar directly related to the login modal and notification modal. Those have now been updated the the stories are back to the regular behavior.

aelkiss

Reviewed with @carylwyatt ; all looks fine.

aelkiss · 2025-09-10T15:06:56Z

src/js/components/Navbar/Navbar.stories.js

-      props: { loggedIn: false, notificationData: null },
-    }),
-  ],
+  // decorators: [


@carylwyatt says this change is because props weren't getting overridden in each individual story; duplicating this in each of them isn't ideal but this is all likely to change with storybook 9 anyway.

aelkiss · 2025-09-10T15:09:57Z

src/js/components/RoleSwitchModal/index.svelte

                    </span>
                    <div class="option--help">
                      {#if switchableRole === 'resourceSharing'}
-                        <p>


This looks great!

aelkiss · 2025-09-10T15:12:14Z

src/js/components/RoleSwitchModal/index.svelte

-    color: var(--color-primary-700);
-    &:hover {
-      color: var(--color-primary-800);
+  .switch-roles :global {


:global avoids warnings about unused css

carylwyatt force-pushed the ETT-266-svelte5 branch from f793fef to 200de31 Compare June 9, 2025 20:45