DockYard / ember-admin

Admin backend for ember-cli projects
MIT License
241 stars 37 forks source link

Breaks with Ember 1.8.0 #26

Closed Globegitter closed 10 years ago

Globegitter commented 10 years ago

When trying to create a new model you are getting the following error: This seems to be related to switching to the morph library rather than the metamorph.

Uncaught TypeError: Cannot read property 'pushChildView' of null vendor.js:52755
merge.appendChild   vendor.js:52755
CoreView.extend.appendChild vendor.js:54535
simpleBind vendor.js:19569
bindHelper   vendor.js:19700
(anonymous function) client/helpers/property-print.js:8
bindView.normalizedValue  vendor.js:19255
SimpleHandlebarsView.render vendor.js:22251
EmberRenderer_createElement  vendor.js:50970
Renderer_renderTree vendor.js:22629
merge.ensureChildrenAreInDOM  vendor.js:52362
View.extend._ensureChildrenAreInDOM vendor.js:52327
DeferredActionQueues.invoke  vendor.js:12725
DeferredActionQueues.flush vendor.js:12777
Backburner.end  vendor.js:12183
Backburner.run vendor.js:12238
apply vendor.js:31331
run vendor.js:29906
hash.success  vendor.js:63258
fire vendor.js:3184
self.fireWith vendor.js:3296
done vendor.js:8362
(anonymous function) vendor.js:8709
rwjblue commented 10 years ago

Issue is with makeBoundHelper. Should be able to fix tonight.

bcardarella commented 10 years ago

@rwjblue ember 1.8 issue or ember-admin?

vasilakisfil commented 10 years ago

is this fixed? I got the same error :/

bcardarella commented 10 years ago

@rwjblue can you respond to this please?

josepjaume commented 10 years ago

Just a heads up - I'm having the same issue.

rwjblue commented 10 years ago

@bcardarella and I worked out a solution yesterday, hopefully get it out there soonish

bcardarella commented 10 years ago

I rolled back those changes, I think I'll wait on 1.8 getting fixed

rwjblue commented 10 years ago

@bcardarella - We are on the last beta with no blocking issues. I doubt the behavior will be changed before 1.8.0 is released.

bcardarella commented 10 years ago

@rwjblue so you guys are going to ship with a backwards incompatible change? That seems wrong.

rwjblue commented 10 years ago

@bcardarella - then report an issue with a reproduction ;)

bcardarella commented 10 years ago

I don't think that's a very good response. You guys know it's broken and you're going to ship broken.

On Wednesday, October 15, 2014, Robert Jackson notifications@github.com wrote:

@bcardarella https://github.com/bcardarella - then report an issue with a reproduction ;)

— Reply to this email directly or view it on GitHub https://github.com/dockyard/ember-admin/issues/26#issuecomment-59252968.


Brian Cardarella CEO of DockYard Visit us: http://dockyard.com Call us: (855) DOCK-YRD Follow me on Twitter: http://twitter.com/bcardarella Follow us on Twitter: http://twitter.com/DockYard

rwjblue commented 10 years ago

I don't think that's a very good response.

Agreed, I apologize.

You guys know it's broken and you're going to ship broken.

I totally agree, we should fix this if we can. The problem is more one of available time than desire. I'll try to repro in a simpler JSBin and at least figure out how big of an issue it is.

stefanpenner commented 10 years ago

You guys know it's broken and you're going to ship broken.

This is why we have betas, but as @rwjblue mentioned time is a big factor. If we can get a failing test or an isolated repo we can likely fix this quickly. Every little bit of help here helps us a lot.

Unfortunately we won't stop the release train, but if such a reproduction exists and the issue is flagged as blocking(i can flag it as such if/when posted) someone will make sure its fixed before the release.

rwjblue commented 10 years ago

I found the root cause of the issue, and am working on sending in a PR to Ember to make the debugging much simpler, but the tldr; is that you cannot have a bound helper add additional views. In our case we had Ember.makeBoundHelper (which creates a SimpleHandlebarsView), and then calling the {{bind}} helper which essentially tries to make another view inside the view created by the makeBoundHelper.

rwjblue commented 10 years ago

Thanks for being patient while we sort this out, and I am sorry again for the overly harsh responses earlier.

bcardarella commented 10 years ago

@rwjblue so was my original code exploiting a bug which is why it worked in the first place?

rwjblue commented 10 years ago

There were apparently many bugs related to nested views from within SimpleHandlebarsView, so when the view layer was re-written the "no children" thing was enforced (by making the view.buffer null inside SimpleHandlebarsView).

So, I guess it was either a bug or a really poorly executed feature (makeBoundHelper was definitely never intended to be used with things that have nested child views), but it was super obnoxious to track down exactly what was happening (many layers of the onion involved)...

I am nearly finished with a PR to Ember (that should make it into beta before 1.8 is released) that makes tracking this down drop-dead simple (with a nice error at the spot where you actually try to nest things).

bcardarella commented 10 years ago

Ok, my concern from earlier still seems valid. Even if this was technically a bug that has been resolved I suspect people may be depending upon it in the same way that I have. You could treat the metal-views in 1.8 as a bug fix and that still satisfies SemVer but there will have to be some good communication about the implications and potential workarounds.

rwjblue commented 10 years ago

Completely agreed, I am still testing my changes, but they will included a [BREAKING BUGFIX] label in the changelog (and I'll make sure to call it out in the blog post).

I'm also going to pester Kris to see if we can just support the old behavior with a deprecation or something (although I suspect the answer is that it will be too difficult to pass things through all the layers properly).

rwjblue commented 10 years ago

I went through the first 1/4 of the search results for makeBoundHelper on GitHub (https://github.com/search?p=25&q=makeBoundHelper&type=Code&utf8=%E2%9C%93), and all were using and returning strings.

Not really sure if that is helpful or conclusive though...

bcardarella commented 10 years ago

Probably, I guess I just did it wrong :p

jeffreybiles commented 10 years ago

Using ember 1.8 and ember-admin 0.0.8 and I'm getting this error. Is there a known fix, or should I wait to use ember-admin?

rwjblue commented 10 years ago

@jeffreybiles - This was fixed in https://github.com/dockyard/ember-admin/pull/39.

@bcardarella - Can you bump and release?

bcardarella commented 10 years ago

@jeffreybiles can you try master branch to confirm the fix then I can release?

jeffreybiles commented 10 years ago

Confirmed, master branch works.

On Thu, Nov 6, 2014 at 8:52 PM, Brian Cardarella notifications@github.com wrote:

@jeffreybiles https://github.com/jeffreybiles can you try master branch to confirm the fix then I can release?

— Reply to this email directly or view it on GitHub https://github.com/dockyard/ember-admin/issues/26#issuecomment-62088861.

bcardarella commented 10 years ago

@jeffreybiles published

jeffreybiles commented 10 years ago

Thank you!

rwjblue commented 10 years ago

@bcardarella - Can you push your version bump commit and tag up?