search

runspired / ember-run-raf

Use requestAnimationFrame with Backburner
MIT License
26 stars 15 forks source link

fix this.import() issue for nested addons #14

Open rox163 opened 7 years ago

rox163 commented 7 years ago

@nathanhammond tried to implement loop needed for nested addons mentioned here https://github.com/runspired/ember-run-raf/pull/13#issuecomment-240837870 @runspired

nathanhammond commented 7 years ago

Note: @stefanpenner and I had a long conversation about this, it's very likely that the recommendation here will change.

For now I recommend you directly copy out the _findHost function from Ember CLI and invoke that as a function instead of inlining it to make sure that this is easier to refactor in the future.

rox163 commented 7 years ago

@nathanhammond refactored.

stefanpenner commented 7 years ago

@nathanhammond sorry, I'm confused why are we allowing/encouraging this?

The work-around that will have the least caveats, is to install this add-on as a dependency of the app. This can be done via a blueprint automatically on ember install

rox163 commented 7 years ago

The issue is smoke-and-mirrors doesn't work when installed via ember install in a routable engine. The error is that this.app is undefined and it points to this dependency as the cause. Thats where this whole thing started.

runspired commented 7 years ago

Also, just a note, smoke-and-mirrors no longer installs or depends on this library.

rox163 commented 7 years ago

oh thats good to know @runspired ! That might just resolve our issue altogether. Version 0.6.x + of smoke-and-mirrors?

nathanhammond commented 7 years ago

@stefanpenner I'm not encouraging this in particular but as a workaround this is the least bad of the options at this point in time. This addon already tried to check and do the "right" thing for .import and it didn't work correctly with nested addons.

@rox163 was paying attention early in the ember-engines world and has made a few PRs throughout the ecosystem to make addons "work" but with the usual version-collision side-effects.

Until we have a better proposal I don't mind this, I've also aimed to make it as easy to remove as possible.

runspired commented 7 years ago

@rox163 confirm on s&m versions that work without ember-run-raf