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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename store and adapter for Ember Data compatibility #454

Closed

Conversation

nolaneo
Copy link
Contributor

@nolaneo nolaneo commented Jan 25, 2017

@intercom we're using both Ember Model and Ember Data in production. We're in the process of migrating fully to Ember Data. While that migration is happening, we have renamed the Ember Model store and data adapter to allow us to use both addons simultaneously as they conflict otherwise.

In this PR we've renamed store to emstore and data-adapter to em-data-adapter.

@ebryn This is a breaking change, but it might be something for you to consider. It opens a path for people to move to Ember Data without having to rewrite all their models at once. Let me know your thoughts. 馃檪

@nolaneo nolaneo force-pushed the en/0124-rename-store-and-adapter branch 2 times, most recently from 271e098 to 2f25707 Compare February 2, 2017 13:23
@GavinJoyce
Copy link
Collaborator

GavinJoyce commented Feb 2, 2017

This is a breaking change

@nolaneo perhaps you could outline the ways in which this is breaking so that we can consider what might be the best path forward?

@nolaneo nolaneo force-pushed the en/0124-rename-store-and-adapter branch 2 times, most recently from dd20b23 to 99fc2a4 Compare February 2, 2017 13:31
@nolaneo nolaneo force-pushed the en/0124-rename-store-and-adapter branch from 99fc2a4 to a97841e Compare February 2, 2017 13:31
@nolaneo
Copy link
Contributor Author

nolaneo commented Feb 2, 2017

In the current release of Ember Model, you create and query records directly through the store by using this.store.createRecord and this.store.find.

For example

let user = this.store.createRecord('user', {name: "Eoin"});
...
let user = this.store.find('user', 1).then(() => ...);

This clashes with Ember Data's usage of this.store.

With this proposed change, you would be required to rename any existing calls to the Ember Model store with the updated naming.

The above example would become

let user = this.emstore.createRecord('user', {name: "Eoin"});
...
let user = this.emstore.find('user', 1).then(() => ...);
...
// Which then allows us to do something like:
let emberDataModel = this.store.createRecord('my-ember-data-model', { ... });

@GavinJoyce
Copy link
Collaborator

GavinJoyce commented Feb 2, 2017

Thanks.

If we'd like to avoid breaking compatibility, we could possibly make the name of the store configurable with a default of store and include instructions on how to enable ember-model and ember-data to work together.

Another option might be to run the ember-model initializer after the ember-data intializer and use a name of emstore if ember-data is present.

@darylalexjohnson
Copy link

First off, thanks @GavinJoyce Big thumbs up on this PR as a way to migrate off of ember-model to ember-data. We will be pushing this to a production environment in the coming weeks.

@bmac
Copy link

bmac commented Feb 2, 2017

One breaking change that may not be obvious from this change is if a user is relying on Ember Route's to automatically find a record when the param follows the pattern <modelname>_id it won't work with Ember Model unless the store is the store property on the Route. See: https://github.com/emberjs/ember.js/blob/ef8f46e15d920dfbe008594a0177a9a2103f90ed/packages/ember-routing/lib/system/route.js#L1612-L1615.

Example:

Router.map(function() {
  this.route('photos', function(){
    this.route('edit', { path: '/:photo_id' });
  });
});
{{#link-to "photos.edit" 1}}
// photos.edit route
export default Ember.Route.extend({
  // This method does not need to be defined since Ember will do this automagically.
  // In practice relying on this behavior is outdated but it still may be used by some older Ember Apps
  model(params) {
    return this.store.find('photo', params.photo_id)
  }
})

application.register('store:main', store);
var emstore = application.Store || Ember.Model.Store;
application.register('emstore:application', emstore);
application.register('emstore:main', emstore);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to discourage this change just for information purposes, as of 2.0.0 Ember Data no longer registers anything on store:main or store:application instead Ember Data registers its store to service:store.

@GavinJoyce GavinJoyce changed the title [Proposal] Rename store and adapter for Ember Data compatibility Rename store and adapter for Ember Data compatibility Feb 22, 2018
@GavinJoyce
Copy link
Collaborator

Closing in favour of #474. I'm going to see if we can land this soon

@GavinJoyce GavinJoyce closed this Feb 22, 2018
@GavinJoyce GavinJoyce deleted the en/0124-rename-store-and-adapter branch February 22, 2018 11:45
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

Successfully merging this pull request may close these issues.

None yet

4 participants