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

SailsSocketAdapter doesn't work with Sails 0.10.0-rc5 #21

Open
nmec opened this issue Apr 17, 2014 · 10 comments
Open

SailsSocketAdapter doesn't work with Sails 0.10.0-rc5 #21

nmec opened this issue Apr 17, 2014 · 10 comments

Comments

@nmec
Copy link
Contributor

nmec commented Apr 17, 2014

There seem to be a number of issues with the SailsSocketAdapter. At first it was throwing an error that socket was not defined, after setting window.socket = io.socket it was unable to find my models until I pluralised them in Ember - which previously worked as singular with SailsRESTAdapter.

I'm using the following stack:

Sails: 0.10.0-rc5
Ember: 1.5.0
Ember Data: 1.0.0-beta.7+canary.b45e23ba 
Handlebars: 1.3.0
jQuery: 2.1.0
@bmac
Copy link
Owner

bmac commented Apr 18, 2014

Thanks for reporting this. I'm not currently using Sails 0.10 on any projects but I will try to set up a demo project today and debug this.

@bmac
Copy link
Owner

bmac commented Apr 21, 2014

I updated the adapter to use io.socket instead of socket. I was also running into a problem in sails.io.js when Ember is extending the array prototype.

As for pluralising the models you are correct that SailsSocketAdapter and SailsRESTAdapter are treating them differently when generating a URL and we should try to make this consistent.

What are your thoughts on the default (singular vs plural) model names for the url?

I think the "ember way" is to use plural model names in the url but in my experience using a singular model name in sails will result in a singular model name in the url. (I recall seeing an option to pluralize it in sails but I couldn't find it with a quick search).

@nmec
Copy link
Contributor Author

nmec commented Apr 22, 2014

My instinct is to match Ember Data and use plural URLs, this is also consistent with the JSON API spec which I believe both projects a standardising around.

The setting you are looking for is the pluralize property in config/blueprints.js.

@guernica0131
Copy link

Hello, @nmec I think I disagree. Though ember has a tendency to pluralize routes, Sails on the other hand disables pluralizes routes by default. Perhaps, having these defaults match might avoid potential confusion. You can turn on pluralized routes by going to conifg/blueprints.js and setting the "pluralize" variable to true.

@bmac
Copy link
Owner

bmac commented May 1, 2014

Hmm @guernica0131 has a point about reducing confusion. If we use pluralizes routes by default in the adapter then we require everyone who wants to use pluralizes routes to set a flag in sails and every one who wants to use singular routes to extend the SailAdapter.

If we go with the singular routes. Dropping the SailAdapter into a project should "just work". The downside is people who want use use plural routes must now make 2 changes (1 in conifg/blueprints.js and 1 in the SailAdapter).

@nmec
Copy link
Contributor Author

nmec commented May 2, 2014

After some digging around the issues on the Sails and JSON-API GH projects it seems that while the JSON-API guys are firmly in the plural camp, Sails is a little more undecided on it's blueprint routes and responses.

It seems like there isn't a perfect solution right now, but as I see it:

  • Long term - Both projects are standardising around JSON-API, so the adapter should aim to match this when the two projects are eventually consistent.
  • Medium term - What configuration should developers use to use pluralised routes? This should be added to the documentation.
  • Short term - This is an adapter so it should aim to make developers lives easier with sane defaults and minimum configuration.

It's worth bearing in mind that Ember may not be the only API client, hence my motivation for this to be standardised around a spec.

@bmac
Copy link
Owner

bmac commented May 5, 2014

@nmec @guernica0131 I have opened a pull request #25 where I refactored both SailsRESTAdapter and SailsSocketAdapter to extend from Ember Data's built in RESTAdapter. This way it will be easier to replace one adapter with the other and have things "just work". I have also made it so both adapters use singular routes and I added some notes to the readme about how to make pluralized routes work.

In a somewhat unrelated question. What Serializers are you folks using with your sails backends?

@bmac
Copy link
Owner

bmac commented May 13, 2014

Ping @nmec @guernica0131. I'd love to hear your feedback on #25 and I'm curious about what Ember Data serializers are you guys are using.

@guernica0131
Copy link

Hi bmac, sorry for the delayed response. It has been busy. To answer your question on #25, it looks like you've found a nice solution. My project has come to a standstill, so I have yet to test and work with the update. I will do so soon, however. In terms of serializers, I haven't gotten that far, but I am certainly open to suggestions. I'll contribute more when my project ramps up again. Thanks!!!

@bmac
Copy link
Owner

bmac commented May 14, 2014

Thanks for the feedback guernica0131. I ended up merging the pr to fix an issue another developer was having with the project.

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

3 participants