feathersjs-ecosystem / feathers-localstorage

A client side service based on feathers-memory that persists to LocalStorage
MIT License
38 stars 15 forks source link

Using without explicitly passing a storage engine is throwing an error with Webpack/Browserify #3

Closed ekryski closed 8 years ago

ekryski commented 8 years ago

@naptiv reported this in https://github.com/feathersjs/feathers-authentication/issues/84:

Btw, I had to change .use('storage', localstorage({"storage":memory})) as I got a ReferenceError: window is not defined at new LocalStorage @feathers-localstorage/lib/index.js:32:41

ekryski commented 8 years ago

@naptiv are you using webpack or babel? I'm wondering if it is a shimming issue.

Maybe we shouldn't be expecting window to just be there and instead you always get the developer to pass in their storage engine explicitly. cc/ @daffl

ghost commented 8 years ago

@ekryski webpack based on this boilerplate https://github.com/RickWong/react-isomorphic-starterkit

ekryski commented 8 years ago

Ya the issue is that with webpack you need to shim the window object. So you end up with something like this in your config.

externals: [
        {
            "window": "window"
        }
    ],

Not sure how I want to proceed here. I think it actually might be more flexible and work "out of the box" better if we explicitly get people to pass in their storage engine. Less magic.

daffl commented 8 years ago

We should probably also set https://github.com/feathersjs/feathers-localstorage/blob/master/src/index.js#L7 to

this._storage = options.storage || (typeof window !== 'undefined' && window.localStorage);
daffl commented 8 years ago

And then throw an error if this._storage is still undefined.

ekryski commented 8 years ago

Ya I think you're right @daffl