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

localStorage throwing error: Should wrap property of object #1129

Closed
laumair opened this issue Aug 15, 2016 · 4 comments
Closed

localStorage throwing error: Should wrap property of object #1129

laumair opened this issue Aug 15, 2016 · 4 comments

Comments

@laumair
Copy link

laumair commented Aug 15, 2016

  • "sinon": "^1.17.4",
  • Environment : mocha
  • Libraries: Enzyme+SinonChai+chai
render() {
    let name = (_.get(currentUser, 'name') || _.get(localStorage, 'username'));
return();
}

This is a piece of code in my component JSX that I am trying to test.
So with this,

 it('should fetch from local storage', () => {
      const props = {
        currentUser: {}
      };
      const store= sinon.stub(window.localStorage, 'setItem').withArgs('hello', 'nothing');
      expect(store.calledOnce).to.equal(true);
      window.localStorage.restore();
    });

gives an error: TypeError: Should wrap property of object

I've gone through this #1046 but I'm guessing this was fixed in the version I'm using.
At first I thought it was some personal implementation error because previously when I had to access window I had create a global object that mocked window:

  const global = {
          window: {
            location: 'git@git'
       }
    };
    const wrapper = mount(<Topbar {...props} />);
   wrapper.find('section').children().simulate('click');
    expect(global.window.location).equal('git@git');

I tried this approach but, that isn't the root of my current problem as far as I think. I've traced back the error but so far no luck.

[Windows/Chrome]

@fatso83
Copy link
Contributor

fatso83 commented Aug 15, 2016

I think this is a duplicate, see #1046. I think the fix was introduced in #1098. That is only present in the 1.17.5 release and the master branch. Retest using either.

PS. Stubbing of localStorage is not possible across browsers. What might work in Chrome today fails in Firefox and might fail in the Chrome of tomorrow. Test using a higher abstraction and use DI.

@laumair
Copy link
Author

laumair commented Aug 16, 2016

@fatso83 Thank you. I'll update to 1.17.5.
I managed to make it work on Chrome with a possible workaround.
Note I am using JSdom and I think it doesn't currently provides localStorage. So, I faked localStorage in my Jsdom configuration file.

if (!global.window.localStorage) {
  global.window.localStorage = {
    getItem() { return '{}'; },
    setItem() {}
  };
}

Test

it('should fetch from local storage', () => {
      const props = {
        currentUser: 'UMAIR',
        user: {
          is_key: false
        }
      };

      const spy = sinon.spy(global.window.localStorage, "setItem");
      spy(props);
      expect(spy.calledWith( {
        currentUser: 'UMAIR',
        user: {
          is_key: false
        }
      }));
      spy.restore();

      const stub = sinon.stub(global.window.localStorage, 'getItem');
      stub(props);
      expect(stub.calledWith(Object.keys(props)));
// stub.restore();
    });

The tests pass this way. You mentioned using a higher abstraction as DI. Can you share an example what you referred to?

@laumair laumair closed this as completed Aug 16, 2016
@laumair laumair reopened this Aug 16, 2016
@fatso83
Copy link
Contributor

fatso83 commented Aug 16, 2016

@umairwanclouds: re higher abstraction I was simply thinking of instead of directly interfacing with localStorage in your code, you create a thin abstraction layer. Say a Storage object:

function Storage(opts) {
     if(opts.db){ this.db = opts.db }
     else { this.db = window.localStorage; // default } 
}
Storage.prototype = {
   get : function(key) { return this.db.getItem(key); }
   set : function(key,val) { return this.db.setItem(key, val); }
}

Then you test against this abstraction, instead of at the lowest implementation level.

describe('storage', function(){
    it("should call the underlying db implementation's getItem", function() {
         var dbStub = { getItem : sinon.stub(); }
         var storage = new Storage(dbStub);
         storage.getItem('foo');
        assert.true(dbStub.getItem.calledOnce);
    });
});

P.S. The code above is not testet, but should illustrate the point.

@fatso83 fatso83 closed this as completed Aug 16, 2016
@laumair
Copy link
Author

laumair commented Aug 16, 2016

@fatso83 Ah I see. That's much better. Thanks a lot.

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

No branches or pull requests

2 participants