sir-dunxalot / ember-easy-form-extensions

Manages form submission in the controller/component and route layers of Ember apps
MIT License
28 stars 14 forks source link

this.get(...).trigger is not a function #29

Closed alvinvogelzang closed 9 years ago

alvinvogelzang commented 9 years ago

Hi I tried your 1.11 branch but on every page I got the error: this.get(...).trigger is not a function

It comes from the routing-event.js intializer and happens when this code is triggered: this.get('controller').trigger('routeDidTransition');

screen shot 2015-06-02 at 14 56 01

When I use your master branch the error does not occur. Any idea what's going wrong here?

I used Ember 1.12.1.

sir-dunxalot commented 9 years ago

It would appear that the Ember.Controller.reopen in addon/initializers/routing-events.js is not adding Ember.Evented correctly.

That aforementioned routing events initializer is really only used in this mixin. I've never really been happy with the initializer being a part of this addon because it adds unnecessary fluff to the rest of the consuming app. If there was a better was to run a method in the controller when the route transitions (maybe calling this.get('controller').methodName() on the route willTransition or didTransition event) that would be a superior solution and would undoubtedly fix this issue.

I haven't seen this issue locally but unfortunately the 1.11 branch doesn't have testing coverage.

Unfortunately I don't have time to get to this within the next couple of days but I'd happily accept a PR.

sir-dunxalot commented 9 years ago

A good fix would be to add a mixin for routes that does the above calling. For example:

// mixins/routes/form-events.js

import Ember from 'ember';

export default Ember.Mixin.create({
  addValidationObservers: function() {
    this.get('controller').addValidationObservers();
  }.on('didTransition'),

  removeValidationObservers: function() {
    this.get('controller').removeRevalidationObservers();
  }
});

Then, in mixins/controllers/saving.js:

... Or something like that. Maybe one could combine this with the basic-dirty-handler mixin? Either way, then the initializer/routing-events.js file could be removed from this addon entirely.

joostdevries commented 9 years ago

I'm curious why you've chosen to implement Submitting on the View. In general I avoid touching those unless I have some craze jq stuff I need to hack.

sir-dunxalot commented 9 years ago

Three reasons:

I think the second point is the strongest - it's the only real way to intercept the saving/cancel action in the view without hacking it from the controller. Perhaps the solution here would be to overwrite the action on formSubmission to target the view if the dev really needs to capture events in the view?

We've been back and forth about refactoring it into one mixin for controllers. I'd be open to that providing there is some mechanism (whether extendible or overwritable) for capturing events in the view on the off chance that is required. The 1.11 branch is very early stage and I'd love to get a really robust API figured out. I agree that it's currently sub-par.

Truffula commented 9 years ago

This occurs for me on routes which use an ArrayController. Work-around in an initializer:

Ember.ArrayController.reopen(Ember.Evented);
sir-dunxalot commented 9 years ago

Ah, I see. The beta branch moved the initializer reopen from Ember.ControllerMixin.reopen to Ember.Controller.reopen due to the deprecation of proxy controllers. So a possible solution could be to revert that change though really everyone using this branch should be seeing deprecation warnings for the proxy controllers (and thus the only controller not throwing warnings now or in the near future will be Ember.Controller). That is, unless I misunderstood the planned deprecation pre-Ember 2.0.

Truffula commented 9 years ago

Right you are, they are being deprecated. Guess I'll have to start working without ArrayControllers then...

sir-dunxalot commented 9 years ago

It's easy to update to Ember.Controller in advance of routable components. All the array controller was really doing was proxying methods from the controller to it's content, which was an array. Thus, make sure any methods proxying to content are run on content or the model directory. For example:

Ember.ArrayController.extend({

  returnSorted: function() {
    return this.sortBy('publishedAt');
  },

});

Would now look like:

Ember.Controller.extend({

  returnSorted: function() {
    return this.get('model').sortBy('publishedAt');
  },

});