Dr. Ember Doctor: tool for diagnosing/improving Ember Apps


#1

I have this idea floating around in my brain about an Ember app sanity checker, perhaps called Ember Doctor, that will look at all your app code and warn you off possible errors/gotchas that might otherwise be tricky/subtle to solve. Perhaps this would be an npm command line tool, or perhaps it’d be a tab in the Chrome Ember Extension, but here are some possible things it could help out with:

Catch Lazy Computed Property Misuse

The lazy computed properties optimization introduced a little before Ember 1.0.0 requires that you “consume” (call .get() on) the dependent keys you specify in a computed property in order for that property to be properly watched for changes. It can be very tricky to debug when you don’t exactly adhere to this rule, and there’s very little you can do other than saying Kris Selden’s name three times in a row until he materializes and points out the likely culprit. It’d be possible (perhaps only for an npm command line utility) to parse your app’s JS code into an AST and check that you specified x as a dependent key but that you don’t actually call this.get('x'), which can lead to subtle errors.

Let you know of nicer/newer Ember patterns

e.g. Ember Doctor could detect that you do something like

var SignupModalView = Ember.View.extend({
  elementId: 'signup-modal',
  didInsertElement: function() {
    $('#signup-modal').something();
  }
});

and then provide the following suggestions

1. Possible refactor: replace `SignupModalView#didInsertElement` with a 
   more descriptive method name, and call `.on('didInsertElement')` on the
   method so that it will run at the same time that `didInsertElement` would,
   e.g.

   moreDescriptiveName: function() {
      // `SignupModalView#didInsertElement` code here...
   }.on('didInsertElement')

2. Refactor: Use `this.$()` rather than `$('#signup-modal')` to get the
   jQuery object associated this this view's element

3. Minor quibble: use `Ember.$` instead of `$` in case your app ever needs to
   function in an environment where `$` is not available as a shorthand for
   `jQuery`

This can also be used for things like suggestion Ember.computed.alias instead of somePropertyNameBinding, or any of the recent patterns that are meant to replace old ones that haven’t been officially deprecated yet.

Suggest when to use Ember.run

At the very least, this could catch setTimeout usage and suggest Ember.run.later instead. Maaaaaybe it could be smart enough to detect when you’re providing a third party library a hook into an Ember Object without wrapping the contents in an Ember.run, though I have many doubts.

Parse Router.map, detect invalid link-to's

Ideally you should have 100% coverage in your test cases, but in case not, this would allow you to catch a failing link-to without actually having to navigate to the route/template containing the link-to to see the error.

Anyway…

Ideally all of this would be doable within Ember Extension; just being able to go to your Ember Extension tab and see a laundry list of possible improvements would be really nice, but there are limitations once code has been minified, or if the check that Ember Doctor needs to make digs into the realm of what can only be done with a JavaScript AST vs just some dumb regex. But either way, whether it’s an Ember Extension tab, an npm package, or both, I’d like to collect some ideas from everyone as to what’s been painful to debug, enlightening to finally discover (such as newer/more refined Ember patterns), and see what the best tool would be. Or maybe this is a crappy idea; I wanna hear that too if that’s the case.


#2

Another idea:

Calling `transitionToRoute` is generally discouraged; it might make
more sense for you to move this action into a Route

findByAttribute inside ObjectController
#4

I wouldn’t want it to just spew out generic how-to information unless it detected something strange you were doing in your app, and passing args to transitionTo isn’t strange.

What exactly gets in the way?


#6

I think this is a great idea. Would be helpful as a part of ember inspector.

Another useful avenue would be some more context provided in the console warnings. For example if there was a consistent way to tag specific warning messages with a unique code this could provide some useful hooks for the ember doctor tool. And provide more context and steps to resolve issues as they come up.

Also, I wouldn’t bother in the minified case. Make it is a feature of debug builds only. For a few reasons

1.) Probably is not helpful for ember inspector spewing all the dirty laundry of an app in production.

2.) As you suggest, limitations in parsing and introspection.

3.) Probably easier to inject additional hooks if necessary. Meaning that these hooks are stripped out minified builds of the code.

What about the idea of providing users special hooks to debug their code. Not sure exactly what this would look like but perhaps some specific way to be declarative and express debug intent, and provide additional hinting to the ember doctor analysis. The idea being “I think there might be some issues with this part of my code can you take a look?” Perhaps in that scenario the diagnostic tool could be more aggressive or pay extra attention to flags that the user specifies.

As far as what is the most painful to debug it is sometimes just the silliest stuff. When an explcitily defined controller, route, etc is not following a naming convention or is misnamed it would helpful provide hints at the obvious. Basically this comes down to the naming convention stuff. Having some more context around this would be immensely helpful to people new to the framework.


#7

@eccegordo The naming convention bug-catcher would be mega helpful and probably not too insane to do; if Ember Doctor has access to all the templates/routes/etc, it doesn’t seem too crazy that it’d be able to figure out that you’ve defined a class that isn’t referenced by your app in any way. It’d be really hard to implement, but I think doable?


#8

Conceptually we know about the “generated” objects so perhaps a basic heuristic that keys off this information and compares with explicitly defined objects. Perhaps we get mostly there just knowing what is “expected” to be generated and how it actually differs because of explicitly defined objects.

Not sure where the hooks would go, but perhaps it would be possible to extend some of the resolver and lookup factory logic. That is what feels like the right place.

Perhaps some wizardry with the container.registry could solve the problem.

But admittedly, I am still not yet that familiar with enough of the ember internals, to understand all the implications.


#9

Two other comments:

1.) Behavior around “index” routes can be a little tricky to get a handle on because of their implicit nature. Resource vs Route…

2.) More meaningful help with error messages. For example if I see an error message like this:

There is no route named foo.bar.index

I would like to see something that says:

You need to create

App.FooBarIndexRoute = Ember.Route.extend({ }); ...

Obviously customized to app name space, or module pattern depending on how the code gets created.


#10

This seems like a project that has a scope like that of CodeClimate. I think it would be wonderful to see this. Some ideas that I have are:

  • finding patterns that should be using some form of Ember.computed and let the user know that it is recommended to change their code. For example:
App.UserController = Ember.ObjectController.extend({
  isAdmin: function () {
    return this.get('role') === 'admin';
  }.property('role')
});

// >> Detected computed property that could be reduced to short form:
//  /controllers/user.js
//  2  isAdmin: function () {
//  3    return this.get('role') === 'admin';
//  4  }.property('role')
//
//  can be reduced to:
//  Ember.computed.equal('role', 'admin')

This could apply to all of the macros, since using them usually speeds up your app (especially the reduced computed properties).

  • Code quality. When seeing procedural code, provide a suggestion to make it use property binding (if that’s what should be encouraged)
  • Recommending splitting up files into smaller components, especially if there is duplicate code. (creating mixins, common templates, etc.) A common example might be several routes that have duplicate templates. This tool should provide information on how to have these routes use the same template.
  • Best practices. eg. thin v. fat models / thin controllers / views holding state, etc.

I understand that some of these are a bit crazy, but I just wanted to get some ideas rolling as to the extent of how much of a boon this could be for developing the best ember app possible.


#11

I think a great MVP for this project could be a “Best Practices” page that collects and describes all these tips (for example, in the Ember Wiki). This would require much less code, and still be really useful.


#12

This sounds like an interesting idea! It sounds like this tool could take some inspiration from http://www.foodcritic.io/


#13

Sounds like a great idea. I would prefer it as a command line tool so it could be integrated with CI tools and IDEs. I see this as a handy sublime text plugin, for example.


#14

@machty it would be great if this library could be used in EAK like JSHint but for Ember.

Do you have any further thoughts on when work on this could start?


#15

A great way of figuring out rules and such would be “doctor embering” Discourse (if you are in to this one file at a time in a PR with tests), the code went through so many iterations from first betas of Ember that it has been hard to keep up with all the latest, greatest and fastest ways of doing things.


#16

I just stumbled upon https://github.com/nzakas/eslint which is “A tool for identifying and reporting on patterns in JavaScript.” and “ESLint is completely pluggable, every single rule is a plugin and you can add more at runtime.”.

Sounds interesting to me. Anybody already had time to work on Dr. Ember? :syringe:


#17

ESLint looks very interesting. Have you tried it?


#18

Nope, not yet. But at the first glance it looks really useful.


#19

I’m probably going to try it over the upcoming Holidays. I’ll post what I find.