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

Map some extra angular deps #6173

Merged
merged 2 commits into from
Feb 11, 2016
Merged

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Feb 10, 2016

Plugins created with the plugin generator import the modules they need, but some of their dependents rely on autoloading to function properly. This fixes those un-mapped dependences.

@spalger
Copy link
Contributor Author

spalger commented Feb 11, 2016

Verbal LGTM from @BigFunger

spalger added a commit that referenced this pull request Feb 11, 2016
@spalger spalger merged commit 5d8a97b into elastic:master Feb 11, 2016
@spalger spalger deleted the fix/mapAngularDeps branch February 25, 2016 22:49
@spalger
Copy link
Contributor Author

spalger commented Jul 5, 2016

requested by @tuxtitlan here

The problem this pr was trying to solve is caused by modules not loading their dependencies using import/require statements.

When a module has import 'ui/notify' in it, and that module is included in the build, the optimizer will ensure that the ui/notify module is included as well. This works great until modules like delayed updater declare an angular dependency without importing the related module (in this case, Notifier is injected without including import 'ui/notify'.) This hasn't caused any failures in Kibana because some other file in the Kibana build was already importing the ui/notify module, so angular didn't know that there wasn't a matching import statement in the delayed updater.

Now though, plugin development is picking up and some plugins are only importing a subset of Kibana's modules (noramlly using autoloading). When those authors try to import a service like the delayed updater angular complains because it can no longer find the Notifier dependency. To fix this we have to identify which module is failing to load, what dependency is not being imported, and add an import statement for that dependency.

this is one of the benefits of the Private module loader, it requires the use of import and prevents accidentally forgetting to import services. It's still an issue for styles and directives/filters unfortunately

@spalger
Copy link
Contributor Author

spalger commented Jul 5, 2016

Ironically, here is another case of this happening. This time there was again no error, angular just silently ignored the unknown render-directive element... #7629

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants