btroncone / ngrx-store-localstorage

Simple syncing between @ngrx store and local storage
MIT License
613 stars 118 forks source link

Angular Universal Support #73

Closed Joniras closed 5 years ago

Joniras commented 6 years ago

As i tried out this (really nice and helpful) library i got surprised with the fact, that Angular Universal threw an Error. lcoalstorage is not defined

It seems that this library doesnt support Angular Universal. I think many would appreciate support for it.

I solved it in the meanwhile with:

const customStorage: Storage = {
  length: 0,
  clear: function (): void {
    if (window && window.localStorage) {
      window.localStorage.clear();
      this.length = window.localStorage.length;
    }
  },
  getItem: function (key: string): string | null {
    try{
      return window.localStorage.getItem(key);
    }catch{
      return null;
    }
  },
  key: function (index: number): string | null {
    try {
      return window.localStorage.key(index);
    }catch{
      return null;
    }
  },
  removeItem: function (key: string): void {
    try {
      window.localStorage.removeItem(key);
      this.length = window.localStorage.length;
    }catch{
        return ;
      }
  },
  setItem: function (key: string, data: string): void {
    try{
      window.localStorage.setItem(key, data);
      this.length = window.localStorage.length;
    }catch{
        return ;
      }
  }
};

export function localStorageSyncReducer(reducer: ActionReducer<any>): ActionReducer<any> {
  return localStorageSync({keys: [{auth: ["token"]},{shopping_cart: ["products"]}], rehydrate: true, storage: customStorage})(reducer);
}
whisher commented 6 years ago

@Joniras You are the man :) Thanks I agree with you the library is quite useless without universal support. Come on btroncone :) I've ever used your libs without any issues

k-schneider commented 6 years ago

This also affects the angular-cli "app shell" feature.

magnattic commented 6 years ago

Building on @Joniras solution from above, I simply faked the localStorage for the whole application in my server.ts during the universal startup, similar to how I faked window and document with domino:

const fakeStorage: Storage = {
  length: 0,
  clear: () => { },
  getItem: (_key: string) => null,
  key: (_index: number) => null,
  removeItem: (_key: string) => { },
  setItem: (_key: string, _data: string) => { }
};

const domino = require('domino');

const win = domino.createWindow(template);
(global as any)['window'] = win;
(global as any)['document'] = win.document;
(global as any)['navigator'] = win.navigator;
(global as any)['localStorage'] = fakeStorage;

This way you don't have to change anything in your metareducer. All window.localStorage calls during SSR will simply go against the fake.

paviad commented 6 years ago

I think this is very important

rafa-suagu commented 5 years ago

@btroncone what do you think about this?

bufke commented 5 years ago

Hmm no solution seems ideal. Typically one would inject isPlatformServer into a module/component and disable the functionality that doesn't run on node. The try...catch solution would hide valid errors too. The domino solution is quite heavyweight to just effectively disable some code.

Does anyone know of a way to check the server platform outside of a module? The check needs to happen in the localstorage metareducer or while defining the meareducers.

bufke commented 5 years ago

I think this is a better way to support it. Just add it conditionally to the metareducers if window is undefined. For example:

const metaReducers: Array<MetaReducer<any, any>> = [];
const localStorageSyncReducer = (
  reducer: ActionReducer<any>
): ActionReducer<any> => {
  return localStorageSync({ keys: ["foo"], rehydrate: true })(reducer);
};
if (typeof window !== "undefined") {
  metaReducers.push(localStorageSyncReducer);
}

If this works for people - I think the best solve is to just document not to add the meta reducer when running in node. I can't think of any reason someone would want localstorage sync to happen server side.

Update never mind this doesn't work. It works without --prod but localstorage sync just doesn't happen with --prod. I tried rearranging it and it seems no method that involves conditionally adding the metareducer works.