Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat/implement local storage persistence - WF-93 #687

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

FabienArcellier
Copy link
Collaborator

The application can save values ​​in local storage and retrieve them when the user reloads their page. This pattern allows you to introduce persistence beyond the current session.

implement #578

Peek 2024-12-11 08-37

@FabienArcellier FabienArcellier added the enhancement New feature or request label Dec 11, 2024
@FabienArcellier FabienArcellier self-assigned this Dec 11, 2024
@FabienArcellier FabienArcellier force-pushed the feat/WF-93-implement-local-storage branch from 87daa7d to 896cb5e Compare December 11, 2024 07:55
@FabienArcellier FabienArcellier changed the title feat/implement local storage - WF-93 feat/implement local storage persistence - WF-93 Dec 11, 2024
@FabienArcellier FabienArcellier force-pushed the feat/WF-93-implement-local-storage branch from 896cb5e to e27f527 Compare December 23, 2024 13:27
Comment on lines 78 to 90
const localStorageItems = {};
for (let i = 0; i < localStorage.length; i++) {
const key = localStorage.key(i);
if (key.startsWith("wf.")) {
const value = localStorage.getItem(key);
localStorageItems[key.replace("wf.", "")] = JSON.parse(value);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

If one value is not valid JSON, it'll throw and stop the execution of the initSession. It might be better to catch it and remove the entry if so.

It could be something like

Object.entries(localStorage)
  .filter(([key]) => key.startsWith("wf."))
  .reduce((acc, [key, value]) => {
    try {
      acc[key.replace('wf.', '')] = JSON.parse(value)
    } catch {
      localStorage.removeItem(key)
    }
  }, {});

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We decided to remove this mechanism to use an interface closer to that of the browser. WF will only write strings to local storage. All keys will be loaded at startup and transferred to the backend.

I think there is no need to add error checking in this scenario. Am I right ?

Comment on lines 62 to 67
addMailSubscription("localStorageSetItem", (event) => {
localStorage.setItem("wf." + event.key, JSON.stringify(event.value));
});
addMailSubscription("localStorageRemoveItem", (event) => {
localStorage.removeItem("wf." + event.key);
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can add the type

Suggested change
addMailSubscription("localStorageSetItem", (event) => {
localStorage.setItem("wf." + event.key, JSON.stringify(event.value));
});
addMailSubscription("localStorageRemoveItem", (event) => {
localStorage.removeItem("wf." + event.key);
});
addMailSubscription("localStorageSetItem", (event: LocalStorageSetItemEvent) => {
localStorage.setItem("wf." + event.key, JSON.stringify(event.value));
});
addMailSubscription("localStorageRemoveItem", (event: LocalStorageRemoveItemEvent) => {
localStorage.removeItem("wf." + event.key);
});

@FabienArcellier FabienArcellier force-pushed the feat/WF-93-implement-local-storage branch from e27f527 to 5ec15a2 Compare December 23, 2024 14:54
@@ -153,6 +157,19 @@ def __init__(self, session_id: str, cookies: Optional[Dict[str, str]], headers:
def update_last_active_timestamp(self) -> None:
self.last_active_timestamp = int(time.time())

def process_local_storage_changes(self) -> None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not required anymore

@FabienArcellier FabienArcellier force-pushed the feat/WF-93-implement-local-storage branch from 5ec15a2 to d6d7982 Compare December 23, 2024 15:04
* feat: propagate local storage into session to keep it into sync
@FabienArcellier FabienArcellier force-pushed the feat/WF-93-implement-local-storage branch from d6d7982 to e57612c Compare December 23, 2024 15:09
@FabienArcellier FabienArcellier marked this pull request as ready for review December 23, 2024 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants