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
Make our output JS less terrible. #31
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going all in with ES6 we should probably do a major version bump just in case anyone else is using braise.
Also what do you think of publishing our eslint standards?
const modelName = this.constructor.modelName; | ||
const adapter = this.store.adapterFor(modelName); | ||
return adapter.autocomplete(modelName, this.get('id'), arguments).then((response) => { | ||
const serializer = _this.store.serializerFor(modelName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is _this
unnecessary now that we're using =>
function ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure the anon function works there with how Ember does things :(. Will test it out.
lib/braise/adapter_template.ex
Outdated
var underscorized = Ember.String.underscore(type); | ||
return Ember.String.pluralize(underscorized); | ||
pathForType(type) { | ||
var underscorized = EmberString.underscore(type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const
?
lib/braise/adapter_template.ex
Outdated
@@ -93,7 +96,7 @@ defmodule Braise.AdapterTemplate do | |||
action_name = link_action.name | |||
method = link_action.method | |||
""" | |||
#{action_name}: function(modelName, id, snapshot) { | |||
#{action_name}(modelName, id, snapshot) { | |||
var url = this.buildURL(modelName, id) + '/#{action_name}'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const
?
test/adapter_template_test.exs
Outdated
@@ -57,12 +60,15 @@ defmodule AdapterTemplateTest do | |||
import DS from 'ember-data'; | |||
import Ember from 'ember'; | |||
|
|||
export default DS.RESTAdapter.extend({ | |||
const { RESTAdapter } = DS; | |||
const { computed, EmberString: String } = Ember; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be the other way
const { computed, String: EmberString } = Ember;
test/adapter_template_test.exs
Outdated
@@ -74,7 +80,7 @@ defmodule AdapterTemplateTest do | |||
return this._super(url, type, options); | |||
}, | |||
|
|||
cancel: function(modelName, id, snapshot) { | |||
cancel(modelName, id, snapshot) { | |||
var url = this.buildURL(modelName, id) + '/cancel'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're ES6'ing all this should we standardize to use let
or const
here (& everywhere else) too?
This reforms our template output so it's a bit more ES6-7 compatible.