Skip to content

Conversation

@spalger
Copy link
Contributor

@spalger spalger commented Apr 27, 2020

We've heard that some people are experiencing Module 'ngSanitize' not available errors caused by running Kibana with a number of plugin's disabled. Unfortunately this is going to happen with Angular modules as it is very easy to have a very implicit dependency on other Angular modules without anyone knowing. Additionally, every module for the most part populates a global module registry making disabling even a few plugins cause these types of errors. Usually the solution is just to make the dependency on specific modules more explicit by putting bare import "<module>" statements in any file that uses that npm module's Angular modules, as seen here for the angular-sanitize npm module and the ngSanitize Angular module.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@spalger spalger marked this pull request as ready for review April 28, 2020 16:57
@spalger spalger requested review from a team April 28, 2020 16:57
@spalger spalger added release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v8.0.0 labels Apr 28, 2020

import angular, { IWindowService } from 'angular';
// required for `ngSanitize` angular module
import 'angular-sanitize';
Copy link
Contributor

Choose a reason for hiding this comment

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

I actually already have this in my NP pr here Maybe this can be removed then? (to avoid merge conflicts). I can just add the comment so it follows the same pattern. wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this will cause merge conflicts if the change is the same, and we probably shouldn't add a dependency on that PR to make this PR "complete"

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the same, but I moved everything outside the legacy directory (and the file name is also different) thus github won't track it as same (and won't do its "auto-merge" magic).

Just thought I should bring it up, so you're not confused with the deleted modules.ts file when trying to merge and resolve conflicts (if my PR goes into master first that is)

@igoristic
Copy link
Contributor

@spalger I'm wondering if we should also do this for ngRoute? via import 'angular-route'

@spalger
Copy link
Contributor Author

spalger commented Apr 28, 2020

@spalger I'm wondering if we should also do this for ngRoute? via import 'angular-route'

I don't see any reason not to, but we should do it in a separate PR if so.

Copy link
Contributor

@igoristic igoristic left a comment

Choose a reason for hiding this comment

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

Looks good 👍 Thank you for adding this!

@spalger spalger merged commit 6986c73 into elastic:master Apr 29, 2020
spalger pushed a commit to spalger/kibana that referenced this pull request Apr 29, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 29, 2020
* master: (60 commits)
  [SIEM] Create template timeline (elastic#63136)
  load react component lazily in so management section (elastic#64285)
  Cleanup .eslingignore and add target (elastic#64617)
  [Ingest] Support yaml variables in datasource (elastic#64459)
  typescript-ify portions of src/optimize (elastic#64688)
  [ngSanitize] add explicit dependencies to all uses of `ngSanitize` angular module (elastic#64546)
  Consolidate downloading plugin bundles to bootstrap script (elastic#64685)
  [Maps] disable edit layer button when flyout is open for add layer or map settings (elastic#64230)
  chore(NA): add async import into infra plugin to reduce apm bundle size (elastic#63292)
  [Maps] fix edit filter (elastic#64586)
  [SIEM][Detections] Adds large list support using REST endpoints
  Replace a number of any-ed styled(eui*) with accurate types (elastic#64555)
  [Endpoint] Recursive resolver children (elastic#61914)
  [ML] Fix new job wizard with multiple indices (elastic#64567)
  Use short URLs for legacy plugin deprecation warning (elastic#64540)
  [Uptime] Update uptime ml job id to limit to 64 char (elastic#64394)
  [Ingest] Fix GET /enrollment-api-keys/null error (elastic#64595)
  Consolidate cross-cutting concerns between region & coordinate maps in new maps_legacy plugin (elastic#64123)
  ES UI new platform cleanup (elastic#64332)
  [Event Log] use @timestamp field for queries (elastic#64391)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 29, 2020
* alerting/np-migration: (64 commits)
  [ML] Changes Machine learning overview UI text (elastic#64625)
  [Uptime] Migrate client to New Platform (elastic#55086)
  Slim vis type timeseries (elastic#64631)
  [Telemetry] Fix inconsistent search behaviour in Advanced Settings (elastic#64510)
  removed unneeded dep and file
  [SIEM] Create template timeline (elastic#63136)
  load react component lazily in so management section (elastic#64285)
  Cleanup .eslingignore and add target (elastic#64617)
  [Ingest] Support yaml variables in datasource (elastic#64459)
  typescript-ify portions of src/optimize (elastic#64688)
  [ngSanitize] add explicit dependencies to all uses of `ngSanitize` angular module (elastic#64546)
  Consolidate downloading plugin bundles to bootstrap script (elastic#64685)
  [Maps] disable edit layer button when flyout is open for add layer or map settings (elastic#64230)
  chore(NA): add async import into infra plugin to reduce apm bundle size (elastic#63292)
  [Maps] fix edit filter (elastic#64586)
  [SIEM][Detections] Adds large list support using REST endpoints
  Replace a number of any-ed styled(eui*) with accurate types (elastic#64555)
  [Endpoint] Recursive resolver children (elastic#61914)
  [ML] Fix new job wizard with multiple indices (elastic#64567)
  Use short URLs for legacy plugin deprecation warning (elastic#64540)
  ...
spalger pushed a commit that referenced this pull request Apr 29, 2020
@spalger spalger deleted the fix/explicit-requirement-for-ngSanitize branch August 18, 2020 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes v7.8.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants