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

fix(ng1): remove the need to trigger digests on promise callbacks #451

Closed
wants to merge 1 commit into from

Conversation

mattlewis92
Copy link
Contributor

Currently using window.angular.injector(['ng']).get('$q'); will create a new angular injector instance which will create a new instance of $q and hence the $rootScope meaning the digests will trigger on a different cycle than the actual app. This is a hacky fix that gets $q via the run block and exposes it globally on the IonicNative global.

@ihadeed
Copy link
Collaborator

ihadeed commented Aug 19, 2016

Attn. @mlynch

@mlynch
Copy link
Collaborator

mlynch commented Sep 1, 2016

Hmm, I don't like adding a variable to IonicNative like that, it feels like the wrong solution to this issue.

Let me dig in a bit and see if there's a different way to depend on $q...

@mattlewis92
Copy link
Contributor Author

You're right, on more thought a better way might be to do:

// in plugin.ts
export const util = {};
util.Promise = Promise;
// in ng1.ts
import {util} from './plugins/plugin.ts';
window.angular.module('ionic.native', []).run($q => {
  util.Promise = $q;
});

This is how the ui-router gets around it.

mlynch added a commit that referenced this pull request Sep 1, 2016
@mlynch mlynch closed this Sep 1, 2016
@mlynch
Copy link
Collaborator

mlynch commented Sep 1, 2016

Hey @mattlewis92, thanks for bringing this one up. I believe this is a more appropriate way to solve this as it grabs the injector from the running app properly: 2dc68a4

Let me know what you think.

@mattlewis92
Copy link
Contributor Author

Will this work if $compileProvider.debugInfoEnabled(false); is set? What if ng-app isn't on the body tag?

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.

3 participants