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

Wrong documentation example for localStorageStore #8312

Closed
fzan opened this issue Oct 26, 2022 · 2 comments
Closed

Wrong documentation example for localStorageStore #8312

fzan opened this issue Oct 26, 2022 · 2 comments

Comments

@fzan
Copy link
Contributor

fzan commented Oct 26, 2022

What you were expecting:
localStorageStore accepting a numeric value as stated in documentation
What happened instead:
Using a numeric value (eg:0) causes store invalidation. The store invalidation calls localStorage.clean that wipe also other data that maybe are unrelated directly with react admin.

Steps to reproduce:

  1. do not use typescript in your project
  2. follow the example from
    https://github.com/marmelab/react-admin/blob/master/docs/Store.md
import { Admin, Resource, localStorageStore } from 'react-admin';

const STORE_VERSION = 2; //<-- this is wrong!

const App = () => (
    <Admin dataProvider={dataProvider} store={localStorageStore(STORE_VERSION)}>
        <Resource name="posts" />
    </Admin>
);

so, because on LocalStorageStore.ts you check
if (storedVersion && storedVersion !== version) {

this is inconsistent because you are saving storedVersion on localStorage as a string, but version in memory is numeric.

note that the session will be invalidated if you refresh the page.

as you declared in localStorageStore.d.ts:
export declare const localStorageStore: (version?: string, appKey?: string) => Store;
With typescript this should trigger a type mismatch

my conclusions are:

  1. the documentation is wrong ( STORE_VERSION should be "2" instead of 2) and this is the original problem
  2. i also want to suggest that you should not clean all the localStorage, (by calling localStorage.clean() ) on version verify, but maybe i can suggest to add a prefix to RaStore variables to clean (for example, "RaStore.myVariable" or "RaStore.2.myVariable") because you cannot be sure that the localStorage doesn't have some custom logic values (as my case) that needs to be preserved 👨‍🔧

Thank you as usual for your great work 🙏

  • React-admin version: 4.4.4
  • Last version that did not exhibit the issue (if applicable):
  • React version: not related
  • Browser: Chrome
  • Stack trace (in case of a JS error):
@fzaninotto
Copy link
Member

Thanks for your ticket!

  1. You're right. Would you like to open a PR to fix the doc?
  2. You're also right. We are indeed using a prefix, so on version change, we should find all keys with that prefix and delete only these one (kind of like what we do in removeItems). Would you like to work on a second PR for this?

@fzaninotto
Copy link
Member

Fixed by #8313 and #8315

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

No branches or pull requests

2 participants