locks / ember-localstorage-adapter

Name says it all.
MIT License
466 stars 156 forks source link

Ember data 2.3+ does not work with this addon #140

Closed Kilowhisky closed 8 years ago

Kilowhisky commented 8 years ago

It seems they've messed with the namespaces for ember data 2.3+. It may no longer be possible to extend the local storage adapter off of the DS namespace.

bmac commented 8 years ago

My guess is the issue here is the fact that DS is no longer a global variable when using Ember Data as an Ember CLI addon. @Kilowhisky if you add "ember-data": "2.3.0" tobower.json` does this addon start working again?

oliverwilkie commented 8 years ago

@bmac I can confirm that a short term of adding "ember-data": "2.3.0" makes this addon start working again.

What would be the long term way to solve this?

bmac commented 8 years ago

Long term ember-localstorage-adapter will likely need to implement a pattern similar to this: https://github.com/pangratz/ember-test-helpers/blob/dad6aed9cd84c98861a125dff501378b93693752/lib/ember-test-helpers/build-registry.js#L100-L110

To attempt to load the Adapter and JSONSerializer out of the require context instead of assuming DS is a global on the page.

Kilowhisky commented 8 years ago

Really this addon should be rebuilt as an actuall ember-cli addon. Probably not too hard to rip the script apart and put it into module form. Then again i'm not entirely sure of the future of this repo.

B-Stefan commented 8 years ago

Actually, I work the transformation into an ember-cli-addon. Most of the work is done but I have some trouble with the integration of the old tests into the new ember-cli-addon test context. I'm not really sure how to handle the old tests. Any idea ?

Should we create a separate issue for this ?

My code is available at https://github.com/b-stefan/ember-localstorage-adapter.git#ember-cli-addon

Kilowhisky commented 8 years ago

I think that if this repo does become a proper addon it'll need to abide by the

import LSSerializer from 'ember-local-storage/serializers/serializer;

syntax by default and maybe have a shim that inserts it into DS.LSSerializer when DS is detected on the global scope.

locks commented 8 years ago

Making LSA a proper addon is my goal, but I've been way more busy than I thought I'd be. I wasn't sure how to handle the tests either :P

I will release a 2.x LSA addon in the future, not retro-compatible. I am studying how best to provide a 1.x release with globals support.

Kilowhisky commented 8 years ago

Another option is to talk with the guy making this addon

https://github.com/funkensturm/ember-local-storage

And maybe talk about a merger?

DanielOchoa commented 8 years ago

Would you accept a PR that makes it into a proper addon for 2.x? This, of course, would include tests.

locks commented 8 years ago

@DanielOchoa I would, but preferably if you coordinate with me in Slack/IRC so I don't have to review the entire process in one sitting.

DanielOchoa commented 8 years ago

Sounds reasonable enough. I'll probably hit you up in slack in the following days.

locks commented 8 years ago

Much appreciated! Talk to you soon.

Kilowhisky commented 8 years ago

Yay! :+1: :heart_eyes:

DanielOchoa commented 8 years ago

https://github.com/locks/ember-localstorage-adapter/pull/141

locks commented 8 years ago

RC1 is out, can you peeps try it out?

DanielOchoa commented 8 years ago

Sure thing

Kilowhisky commented 8 years ago

Looking good in my app!

I was able to remove the "ember-data" from my bower.json too.

oliw commented 8 years ago

Same, many thanks!

DanielOchoa commented 8 years ago

It works as expected on the application I'm using it in.

SmileyJames commented 8 years ago

Looking good for me, much appreciated Daniel.

locks commented 8 years ago

Thanks for the feedback :)