Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace lookup-controller util with Route#controllerFor #38

Closed

Conversation

HeroicEric
Copy link
Contributor

@HeroicEric HeroicEric commented Sep 28, 2017

  • Replaces lookup-controller utility with calls to Route#controllerFor
  • Removes dependency on ember-getowner-polyfill be removed.

The logic in /utils/lookup-controller.js seems to be roughly the same that is
already implemented in Route#controllerFor.

Additionally, this seems to fix a bug that I was running into when
trying to use this addon inside of an engine with factoryFor not being
defined.

Any reason the special util was being used instead of Route#controllerFor?

The logic in `/utils/lookup-controller.js` seems to be roughly the same that is
already implemented in [Route#controllerFor](https://emberjs.com/api/ember/2.15/classes/Ember.Route/methods/controllerFor?anchor=controllerFor).

Additionally, this seems to fix a bug that I was running into when
trying to use this addon inside of an engine with `factoryFor` not being
defined.
@offirgolan
Copy link
Owner

@HeroicEric the reason we do not use this.controllerFor is because it will create the controller prematurely and can cause issues during authentication. Please see #25 for reference.

Is there a reason why factoryFor is not available?

@offirgolan
Copy link
Owner

Closing as this PR has been abandoned. Feel free to reopen if needed.

@offirgolan offirgolan closed this Oct 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants