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

Deprecation warnings about using '<x>Binding' #17

Closed brycekahle closed 9 years ago

brycekahle commented 9 years ago

I'm using this module with Ember v1.11.1 and it works great, but I get a bunch of deprecation warnings in the console like the following:

DEPRECATION: You're attempting to render a view by passing propertyBinding to a view helper, but this syntax is deprecated. You should use `property=someValue` instead.
        at Object.__exports__.default.EmberObject.extend.replaceDeprecate.Ember.deprecate (<anonymous>:981:38)

Any ideas on what code needs to change to fix this? I tried to make some changes myself, but ended up breaking things. If someone has an idea on how to fix this, I'm happy to submit a PR.

jme783 commented 9 years ago

+1, having the same exact issue in my project

sir-dunxalot commented 9 years ago

Thanks for opening the issue. I haven't seen this myself but I think we can get a fix out pretty soon. It might be as simple as changing {{input propertyBinding='view.whatever'}} to {{input property=view.whatever}}. However, the reason these <x>Bindings were added in the first place is because Easy Form displayed unexpected behaviour when not using the Binding syntax. Thus, the fix for this issue might extend into the vendor/easy-form.js file requiring additional changing of the easy form source.

I'm trying to maintain compatibility with the last 2-3 Ember versions.

jme783 commented 9 years ago

@sir-dunxalot thanks for looking into this. I have tried changing propertyBinding to property, and valueBinding to value, and it doesn't appear to do the trick. When I make this change, the bound attribute simply gets evaluated as a string (i.e. view.whatever is displayed as view.whatever in the input).

One idea I had was to use registerBoundHelper instead of registerHelper. That way, we could take advantage of using automatic binding detection. Right now, The library is using /^*Binding/-named properties to manually create Ember.Binding instances as described here. However, this resulted in an error:

Uncaught TypeError: Cannot read property 'container' of undefined. The error gets thrown in the ember-htmlbars viewHelper function here:

var container = view.container || utils.read(view._keywords.view).container;

Digging into this, it appears that the options object that is passed into a registerHelper function are different than a registerBoundHelper function, thus causing this to blow up.

Here's the options object for registerHelper:

screen shot 2015-04-20 at 8 34 40 pm

And for registerBoundHelper:

screen shot 2015-04-20 at 8 39 05 pm

Any ideas that you have on what may be going on would be great. @brycekahle or I seem to both be willing to work to make this work, and submit a P/R

sir-dunxalot commented 9 years ago

Great, thanks for the more detailed explanation. Just to be clear, I didn't add the /^*Binding/ stuff - that's part of Easy Form's source. So it seems like we'll definitely have to dig into the source here.

I would be open to any solution but it sounds like updating Easy Form to use registerBoundHelper might be a solid option here. Or maybe even Ember.HTMLBars.something?

sir-dunxalot commented 9 years ago

Another potential solution might be updating EasyFormShims.getBindingor EasyFormShims.getProperty.

I can't help but wonder if rewriting the source as normal view in Ember CLI's directory structure might work better. At this point we're moving far away from the original source, however, and would probably have to redo a lot of the setup of the wrapper, etc. Then it's basically a rewrite of the Easy Form library (which probably should have happened already, tbh).

sir-dunxalot commented 9 years ago

Sorry for the wall of messages. Perhaps we could even replace some calls to EasyFormShims.getBinding with EasyFormShims.getProperty depending on what's been passed to the Easy Form view.

jme783 commented 9 years ago

All good @sir-dunxalot it's awesome to have a conversation about getting a fix in the works! To me, it seems like replacing the getBindings with getProperty would be a promising start, especially considering the getBinding is appending "Binding" to the propertyName.

I also was poking around with using Ember.HTMLBars.makeBoundHelper. The problem was that when trying this, the helper was no longer being recognized in the template. Here are the currently documented methods available for HTMLBars: http://emberjs.com/api/classes/Ember.HTMLBars.html.

Here's a theory we have here: Although Ember.Handlebars is still available in Ember 1.11, it seems like these methods are really just being wrapped around HTMLBars helpers. Take for example what Ember.Handlebars.registerHelper (what the current library is using) returns in 1.11:

function registerHandlebarsCompatibleHelper(name, value) {
    var helper;

    if (value && value.isHTMLBars) {
      helper = value;
    } else {
      helper = new HandlebarsCompatibleHelper(value);
    }

    helpers['default'][name] = helper;
  }

Would it be possible to take advantage of using HTMLBars directly to fix some of these issues?

Again, these are all scattered thoughts but I figure these may be some helpful clues to point us in the right direction. Thanks!

sir-dunxalot commented 9 years ago

Agreed, dude! It's definitely been a frustrating progress seeing EasyForm stagnate so I'm glad we're making progress here.

I suspect the reason the helper is not being recognized is because it's not dasherized. That means we need to explicitly register the helper using registerBoundHelper after makeBoundHelper or we rename the helpers. For example label --> input-label or hint --> input-hint. It might be that simple and, therefore, would allow us to overcome the initial binding issues.

I don't have much knowledge about HTMLBars and as you saw the documentation is non-existent haha. However, if the HTMLBars package is correctly formatted we might be able to make use of methods like HandlebarsCompatibleHelper. I'm not sure tbh. However, I'm not that bothered about maintaining support for old Ember apps as half the reason this project exists is to support Ember v1.10 and above - would like to hear your thoughts on whether we should support older versions of Ember per-HTMLBars.

I also wonder whether glimmer will affect any of this in the near future - it's expected in Ember v1.13.

I'm knee deep in other work so haven't been able to dig into the code but your progress has been great so far!

sir-dunxalot commented 9 years ago

The binding issue will not be a problem after 1.0.0 is released - bindings have been entirely deprecated as this library is no longer dependent on Easy Form's very outdated vendor file. You can test the 1.0.0 prerelease here. Please note the API for inputs has changed slightly from the original Easy Form syntax.

sir-dunxalot commented 9 years ago

I should add that feedback on the new setup and options will be very much appreciated.

jme783 commented 9 years ago

Hey @sir-dunxalot thank you for this! We will surely be testing this in our Ember app very soon

sir-dunxalot commented 9 years ago

Great, thanks. Hopefully the turn around on issues and feature requests will be very quick - our team will be using it too.

sir-dunxalot commented 9 years ago

37 brings a large upgrade to this addon making it ready for Ember 1.13 and 2.0. That PR fixes deprecation warnings.