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.

Sign up for GitHub

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

Jump to bottom

Namespace Medplum-managed local storage keys #3803

Open
dillonstreator opened this issue Jan 24, 2024 · 1 comment · May be fixed by #3806
Open

Namespace Medplum-managed local storage keys #3803

dillonstreator opened this issue Jan 24, 2024 · 1 comment · May be fixed by #3806
Milestone
Milestone Quality

Comments

@dillonstreator
Copy link
Contributor

dillonstreator commented Jan 24, 2024 •
edited

Currently, calling MedplumClient#clear is quite destructive and blows away all local storage entries regardless of if they're Medplum-managed.

medplum/packages/core/src/client.ts

Lines 805 to 814 in 1ac4758

/**
* Clears all auth state including local storage and session storage.
* @category Authentication
*/
clear(): void {
this.storage.clear();
sessionStorage.clear();
this.clearActiveLogin();
}

This is a side-effect of logging out:

medplum/packages/core/src/client.ts

Lines 1116 to 1120 in 1ac4758

async signOut(): Promise<void> {
await this.post(this.logoutUrl, {});
this.clear();
}

I'd like to propose introducing a prefix to all local storage keys that are created by Medplum to enable #clear to be more surgical and only remove Medplum created/managed local storage entries. Additionally, this would reduce the likelihood of key clashing with other systems/extensions that leverage local storage.
i.e. @medplum:${key}
@medplum:logins
@medplum:activeLogin

This would be a breaking change if it became the default behavior and FWIW, the MedplumClient does already expose storage as an optional input so clients could specify their own local storage wrapper that applies key prefixes and a more surgical removal.
@dillonstreator dillonstreator changed the title Namespace Medplum local storage keys to be more surgical with clearing & reduce likelihood of key clashing Namespace Medplum-managed local storage keys Jan 24, 2024
@dillonstreator dillonstreator linked a pull request Jan 24, 2024 that will close this issue
Open
@dillonstreator
Copy link
Contributor Author

Just noticed that this impacts the work done to enable dark mode
#3814

By default, Mantine leverages the mantine-color-scheme-value local storage key to manage preferred color scheme.
https://github.com/mantinedev/mantine/blob/d1f047bd523f8f36ab9edf3aff5f6c2948736c5a/packages/@mantine/core/src/core/MantineProvider/color-scheme-managers/local-storage-manager.ts#L10
This gets blown away when logging out of https://app.medplum.com

@rahul1 rahul1 added this to the Milestone Quality milestone Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
Medplum Product Roadmap
Status: No status
Milestone
Milestone Quality
Development

Successfully merging a pull request may close this issue.

fix-3803 add prefix to client storage dillonstreator/medplum
2 participants
@rahul1 @dillonstreator