`1931` local storage migration #1996

Da-Colon · 2024-06-13T02:49:19Z

Fixes #1931

End of Series.

This adds the hook useMigrateLocalStorageV1 Which is placed inside the Providers component (at top of tree).

This hook is called when keys that include fract_ for V1 are found. filters for Favorites and combines the networks into a single favorites cache adding the network prefix for network of the favorite.

It then removes all old fract_ keys from local storage clearing out the old cache.

Thoughts

I thought about adding a V1_MIGRATION: true to local storage then use that to make sure this didn't run after running. but felt the search for fract_ should handle this. Thoughts?

- delete other cache

- added conditionals to ensure proper handling of cache

netlify · 2024-06-13T02:49:38Z

✅ Deploy Preview for decent-interface-dev ready!

Name	Link
🔨 Latest commit	`367dbd6`
🔍 Latest deploy log	https://app.netlify.com/sites/decent-interface-dev/deploys/66706b7ad7806b0008d657b5
😎 Deploy Preview	https://deploy-preview-1996.app.dev.decentdao.org
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Da-Colon · 2024-06-13T02:51:14Z

@decentdao/engineering Since this is kinda hard to test (I guess you can add a old favorite cache manually in the deploy preview), I added a test and some test cases for it to make sure this works as expected

adamgall

Two pieces of feedback, which seem in line with your thoughts too:

Yes, let's set up the structure now for multiple migrations. I do think adding a "migration version" to local storage is a good idea at this point.
Just a bit more abstraction please:
- rename the hook used in Providers to useMigrate
- the useMigrate hook checks for existence of "migration version" value
  - if no value exists
    - run the v1 migration
    - set "migration version" to 1
  - if it does exist and is >= 1, don't run the v1 migration

Da-Colon · 2024-06-13T21:58:57Z

Two pieces of feedback, which seem in line with your thoughts too:

Yes, let's set up the structure now for multiple migrations. I do think adding a "migration version" to local storage is a good idea at this point.

Just a bit more abstraction please:

rename the hook used in Providers to useMigrate

the useMigrate hook checks for existence of "migration version" value

if no value exists

run the v1 migration

set "migration version" to 1

if it does exist and is >= 1, don't run the v1 migration

Great ideas, I'll get on it.

src/hooks/utils/cache/useMigrate.ts

test/migrateCacheToV1.test.ts

- use import.meta.glob to get migrations - pass count as param for testing

Da-Colon · 2024-06-16T04:35:47Z

@adamgall Phew. Okay. I realized that I was attempting to use the filesystem (fs) which won't be available in the frontend environment. I considered using a migrations/index to get the file count but didn't like that extra overhead. After some soul searching I discovered import.meta.glob which allows you to dynamically import multiple modules based on glob patterns. This gets us the count we need.

The next problem is that this isn't mockable and since its compiled at runtime can't override it. So added a param for migrationCount that is defaulted to the globs key length, this allows me to now only mock the files in the test but pass in the count for the loop.

tests have been added.

adamgall

Love the tests. Have one question about a small thing, but otherwise looks great

adamgall · 2024-06-17T01:56:27Z

src/hooks/utils/cache/useMigrate.ts

+    try {
+      const migration = await import(`./migrations/${i}`);
+      migration.default();
+      setValue({ cacheName: CacheKeys.MIGRATION }, migrationCount);


I don't think this is correct. Inside each loop iteration, we should set cacheName to i, right?

That is correct. left in a change during testing. Updated that. I also went ahead and removed the localStorage update from the loop. I'd rather do that once on a success or failure.

I also went ahead and added another test for the runMigration button for a successful run (only had error states tested).

- remove setValue from loop;

- fix hoisting issues with mocks

adamgall

Found a bug or two.

And then I have yet other idea about the migration code. should it perform some self-tests?

// dev: as i wrote out the below, proper implementation became more unrealistic with each line. keeping for posterity, but these are just the ramblings of a dreamer for now

For example, consider each migration file was split into 4 distinct steps.

gather all relevant source data
use the source data to CREATE NEW keys only. NEVER modify existing localstorage keys or values in this step.
run tests... make sure new data looks like it should look like, considering the source data.
a. if the tests don't pass then throw an error or something. DON'T DELETE source data or the invalid new data.
b. if they do pass, update migration

src/hooks/utils/cache/migrations/1.ts

src/hooks/utils/cache/useMigrate.ts

src/hooks/utils/cache/migrations/1.ts

test/migrations.test.ts

adamgall · 2024-06-17T15:08:07Z

@Da-Colon i'm pushing one commit. then i'm good with merging 👍

edit 2 mins later: just did a force push @Da-Colon

adamgall

It doesn't seem to work any longer

I don't think it was my commit

adamgall

LESGO

Da-Colon added 5 commits June 12, 2024 17:52

(WIP) migration hook

a17f510

migrate only favorites;

bb3ab2f

- delete other cache

remove old cache keys

774ddab

extract to a testable function;

5f1d82e

- added conditionals to ensure proper handling of cache

add unit test for migration func

2ef4685

Da-Colon requested review from adamgall, DarksightKellar and mudrila June 13, 2024 02:49

Da-Colon self-assigned this Jun 13, 2024

target fract_ more precisely

382d53f

run pretty/lint

fd4e1d8

adamgall reviewed Jun 13, 2024

View reviewed changes

Da-Colon added 2 commits June 13, 2024 18:34

rename -> useMigrate

e9d7792

add migration version

cc402b3

Da-Colon requested a review from adamgall June 13, 2024 22:43

Da-Colon added 4 commits June 13, 2024 21:14

remove old conditional for SSR

62e37d9

set new migration version to track migrations

6a710b2

remove mocking and fix tests for checking migration number

d148f04

run pretty/lint

92bc295

DarksightKellar approved these changes Jun 14, 2024

View reviewed changes

adamgall requested changes Jun 14, 2024

View reviewed changes

Da-Colon and others added 5 commits June 14, 2024 11:39

move cache version check to useEffect

2499cf5

Infinity migrations

04a80e4

fix tests

6e81487

add logger

8a21e7e

rename test

c78e39e

adamgall requested changes Jun 14, 2024

View reviewed changes

src/hooks/utils/cache/useMigrate.ts Outdated Show resolved Hide resolved

test/migrateCacheToV1.test.ts Outdated Show resolved Hide resolved

Da-Colon added 2 commits June 16, 2024 00:28

remove use of fs API

f866df0

- use import.meta.glob to get migrations - pass count as param for testing

update test for malformed filenames

6fb2f02

Da-Colon requested a review from adamgall June 16, 2024 14:05

adamgall requested changes Jun 17, 2024

View reviewed changes

Da-Colon added 2 commits June 17, 2024 00:55

properly set version number based index of loop

8843b83

- remove setValue from loop;

add new test for success migration update.

242361f

- fix hoisting issues with mocks

adamgall requested changes Jun 17, 2024

View reviewed changes

src/hooks/utils/cache/migrations/1.ts Outdated Show resolved Hide resolved

adamgall requested changes Jun 17, 2024

View reviewed changes

test/migrations.test.ts Outdated Show resolved Hide resolved

Da-Colon added 6 commits June 17, 2024 10:50

can delete

492ebff

remove this early exit

3b9191c

early exit if migration is count is same as actualCacheVersion

82d3cf4

add new expect to test before test is run

18a47e4

add never expire param

b495b31

run pretty/lint

d9307e5

Da-Colon requested a review from adamgall June 17, 2024 15:04

Small code tidying up

5626da1

adamgall force-pushed the issue/1931-local-storage-migration branch from d1a0cc9 to 5626da1 Compare June 17, 2024 15:10

adamgall requested changes Jun 17, 2024

View reviewed changes

Da-Colon added 5 commits June 17, 2024 12:05

add file extension

d2e616e

use glob import directly

3cdce28

update tests for new setup

5c10fa7

run pretty/lint

9bb0ff2

remove console log

367dbd6

adamgall approved these changes Jun 17, 2024

View reviewed changes

Da-Colon merged commit 1411e9a into issue/1931-local-storage-updates Jun 17, 2024
7 checks passed

Da-Colon deleted the issue/1931-local-storage-migration branch June 17, 2024 18:16

Da-Colon mentioned this pull request Jun 17, 2024

1931 Part 1: Local storage updates #1973

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`1931` local storage migration #1996

`1931` local storage migration #1996

Da-Colon commented Jun 13, 2024

netlify bot commented Jun 13, 2024 •

edited

Loading

Da-Colon commented Jun 13, 2024

adamgall left a comment •

edited

Loading

Da-Colon commented Jun 13, 2024

Da-Colon commented Jun 16, 2024

adamgall left a comment

adamgall Jun 17, 2024

Da-Colon Jun 17, 2024

adamgall left a comment

adamgall commented Jun 17, 2024 •

edited

Loading

adamgall left a comment •

edited

Loading

adamgall left a comment

1931 local storage migration #1996

1931 local storage migration #1996

Conversation

Da-Colon commented Jun 13, 2024

Thoughts

netlify bot commented Jun 13, 2024 • edited Loading

✅ Deploy Preview for decent-interface-dev ready!

Da-Colon commented Jun 13, 2024

adamgall left a comment • edited Loading

Choose a reason for hiding this comment

Da-Colon commented Jun 13, 2024

Da-Colon commented Jun 16, 2024

adamgall left a comment

Choose a reason for hiding this comment

adamgall Jun 17, 2024

Choose a reason for hiding this comment

Da-Colon Jun 17, 2024

Choose a reason for hiding this comment

adamgall left a comment

Choose a reason for hiding this comment

adamgall commented Jun 17, 2024 • edited Loading

adamgall left a comment • edited Loading

Choose a reason for hiding this comment

adamgall left a comment

Choose a reason for hiding this comment

`1931` local storage migration #1996

`1931` local storage migration #1996

netlify bot commented Jun 13, 2024 •

edited

Loading

adamgall left a comment •

edited

Loading

adamgall commented Jun 17, 2024 •

edited

Loading

adamgall left a comment •

edited

Loading