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

Give access to angular $injector via chrome #15267

Merged
merged 7 commits into from
Nov 30, 2017

Conversation

timroes
Copy link
Contributor

@timroes timroes commented Nov 29, 2017

This PR adds a new API to the chrome, that will give access to the current active Angular $injector, so you can inject Angular services into non Angular related code. This is e.g. done by the visualize loader. Since we are not able to always strip out Angular from the bottom-up, which wouldn't require this, and we already introduced the API (via a Webinar) after some discussion with @spalger we decided to rather create an APi in the chrome for it, but make clear from it's documentation, that you must be aware what you're doing when using Angular services outside of Angular.

Technical details

This method uses the .injector() method provided by Angular on any scoped element on document.body (the same element we bootstrap to). We also wait for bootstrap to run for the case getActiveInjector would be called before the bootstraping starts.

Since we are using the chrome everywhere in Kibana to render the base structure, this will be available from wherever you will call it.

There is just one exception though: Within tests, we don't actually bootstrap the application, but mock the modules. For this case, I've created a fixtures module, which you can use in any test - after you called ngMock.module('kibana') to stub this method, by returning the $injector, that we got via ngMock.inject instead. I am working on another PR that uses this, and it solved all the testing issues.

@timroes timroes added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Visualizations Generic visualization features (in case no more specific feature label is available) chore v6.2.0 v7.0.0 labels Nov 29, 2017
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

}

/**
* This method compines setupInjectorStub and teardownInjectorStub in one method.
Copy link

@rhoboat rhoboat Nov 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/compines/combines

@@ -0,0 +1,44 @@
/**
* This test file contains stubs for chrome.getActiveInjector, that you will
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getActiveInjector -> dangerouslyGetActiveInjector

@timroes timroes merged commit a6b4183 into elastic:master Nov 30, 2017
@timroes timroes deleted the chrome-injector branch November 30, 2017 14:00
timroes added a commit to timroes/kibana that referenced this pull request Nov 30, 2017
* Implement getActiveInjector

* Fix typo in comments

* Rename method to sound more frightening

* Move test utils

* Add more documentation

* Fix typo in comment

* Fix name of method in comments
timroes added a commit that referenced this pull request Nov 30, 2017
* Implement getActiveInjector

* Fix typo in comments

* Rename method to sound more frightening

* Move test utils

* Add more documentation

* Fix typo in comment

* Fix name of method in comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Feature:Visualizations Generic visualization features (in case no more specific feature label is available) Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v6.2.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants