Skip to content

Commit

Permalink
[Visualizations] Remove jQuery from owned plugins + remove flot-chart…
Browse files Browse the repository at this point in the history
…s from shared bundle (#181543)

## Summary

This PR aims to reduce the amount of dependencies used in the
visualizations plugins.

Dependencies removed:
* `jquery` usage in `graph`
* `jquery` usage in `Vega` plugin.

As a bonus point it makes the `@kbn/flot-charts` dependency a Canvas and
Monitoring only (the only 2 consumers) rather than load it for all
Kibana plugins.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
dej611 and kibanamachine authored Apr 29, 2024
1 parent 97e1d9f commit ac5ace0
Show file tree
Hide file tree
Showing 13 changed files with 84 additions and 73 deletions.
1 change: 0 additions & 1 deletion packages/kbn-ui-shared-deps-src/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ webpack_cli(
"//packages/kbn-babel-register",
"//packages/kbn-babel-preset",
# packages included in the shared deps src bundle
"//packages/kbn-flot-charts",
"//packages/kbn-ui-theme",
"//packages/kbn-i18n",
"//packages/kbn-i18n-react",
Expand Down
2 changes: 0 additions & 2 deletions packages/kbn-ui-shared-deps-src/src/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ require('./polyfills');

export const Jquery = require('jquery');
window.$ = window.jQuery = Jquery;
// mutates window.jQuery and window.$
require('@kbn/flot-charts');

// stateful deps
export const KbnUiTheme = require('@kbn/ui-theme');
Expand Down
7 changes: 5 additions & 2 deletions src/plugins/vis_types/vega/public/data_model/url_parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Side Public License, v 1.
*/

import $ from 'jquery';
import { i18n } from '@kbn/i18n';
import { UrlObject } from './types';

Expand Down Expand Up @@ -51,7 +50,11 @@ export class UrlParser {
})
);
} else {
url += (url.includes('?') ? '&' : '?') + $.param(query);
const urlParams = new URLSearchParams();
for (const [name, value] of Object.entries(query)) {
urlParams.append(name, String(value));
}
url += (url.includes('?') ? '&' : '?') + urlParams.toString();
}

obj.url = url;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ export class VegaBaseView {
destroy(): Promise<void>;
resize(dimensions?: { height: number; width: number }): Promise<void>;

_$container: any;
_$controls: any;
_container: HTMLDivElement;
_controls: HTMLDivElement;
_parser: any;
_vegaViewConfig: any;
_serviceSettings: VegaViewParams['serviceSettings'];
Expand Down
81 changes: 49 additions & 32 deletions src/plugins/vis_types/vega/public/vega_view/vega_base_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Side Public License, v 1.
*/

import $ from 'jquery';
import moment from 'moment';
import dateMath from '@kbn/datemath';
import { scheme, loader, logger, Warn, version as vegaVersion, expressionFunction } from 'vega';
Expand Down Expand Up @@ -77,15 +76,15 @@ const getExternalUrlServiceError = (uri) =>

export class VegaBaseView {
constructor(opts) {
this._$parentEl = $(opts.parentEl);
this._parentEl = opts.parentEl;
this._parser = opts.vegaParser;
this._serviceSettings = opts.serviceSettings;
this._filterManager = opts.filterManager;
this._fireEvent = opts.fireEvent;
this._timefilter = opts.timefilter;
this._view = null;
this._vegaViewConfig = null;
this._$messages = null;
this._messages = null;
this._destroyHandlers = [];
this._initialized = false;
this._externalUrl = opts.externalUrl;
Expand All @@ -100,11 +99,13 @@ export class VegaBaseView {

try {
if (this._parser.useResize) {
this._$parentEl.addClass('vgaVis--autoresize');
this._parentEl.classList.add('vgaVis--autoresize');
} else {
this._$parentEl.removeClass('vgaVis--autoresize');
this._parentEl.classList.remove('vgaVis--autoresize');
}
this._$parentEl.empty().addClass(`vgaVis`).css('flex-direction', this._parser.containerDir);
this._parentEl.replaceChildren();
this._parentEl.classList.add('vgaVis');
this._parentEl.style.flexDirection = this._parser.containerDir;

// bypass the onWarn warning checks - in some cases warnings may still need to be shown despite being disabled
for (const warn of this._parser.warnings) {
Expand All @@ -116,23 +117,29 @@ export class VegaBaseView {
return;
}

this._$container = $('<div class="vgaVis__view">').appendTo(this._$parentEl);
this._$controls = $(
`<div class="vgaVis__controls vgaVis__controls--${this._parser.controlsDir}">`
).appendTo(this._$parentEl);
this._container = document.createElement('div');
this._container.classList.add('vgaVis__view');
this._parentEl.append(this._container);

this._controls = document.createElement('div');
this._controls.classList.add(
`vgaVis__controls`,
`vgaVis__controls--${this._parser.controlsDir}`
);
this._parentEl.append(this._controls);

this._addDestroyHandler(() => {
if (this._$container) {
this._$container.remove();
this._$container = null;
if (this._container) {
this._container.remove();
this._container = null;
}
if (this._$controls) {
this._$controls.remove();
this._$controls = null;
if (this._controls) {
this._controls.remove();
this._controls = null;
}
if (this._$messages) {
this._$messages.remove();
this._$messages = null;
if (this._messages) {
this._messages.remove();
this._messages = null;
}
if (this._view) {
const state = this._view.getState();
Expand Down Expand Up @@ -253,18 +260,24 @@ export class VegaBaseView {
}

_addMessage(type, text) {
if (!this._$messages) {
this._$messages = $(`<ul class="vgaVis__messages">`).appendTo(this._$parentEl);
if (!this._messages) {
this._messages = document.createElement('ul');
this._messages.classList.add('vgaVis__messages');
this._parentEl.append(this._messages);
}
const isMessageAlreadyDisplayed = this._$messages
.find(`pre.vgaVis__messageCode`)
.filter((index, element) => element.textContent === text).length;
const isMessageAlreadyDisplayed = [
...this._messages.querySelectorAll(`:scope pre.vgaVis__messageCode`),
].filter((index, element) => element.textContent === text).length;
if (!isMessageAlreadyDisplayed) {
this._$messages.append(
$(`<li class="vgaVis__message vgaVis__message--${type}">`).append(
$(`<pre class="vgaVis__messageCode">`).text(text)
)
);
const messageCodeEl = document.createElement('pre');
messageCodeEl.classList.add('vgaVis__messageCode');
messageCodeEl.textContent = text;

const messageItemEl = document.createElement('li');
messageItemEl.classList.add(`vgaVis__message`, `vgaVis__message--${type}`);
messageItemEl.append(messageCodeEl);

this._messages.append(messageItemEl);
}
}

Expand All @@ -279,8 +292,12 @@ export class VegaBaseView {
}

updateVegaSize(view, dimensions) {
const width = Math.floor(Math.max(0, dimensions?.width ?? this._$container.width()));
const height = Math.floor(Math.max(0, dimensions?.height ?? this._$container.height()));
const width = Math.floor(
Math.max(0, dimensions?.width ?? this._container.getBoundingClientRect().width)
);
const height = Math.floor(
Math.max(0, dimensions?.height ?? this._container.getBoundingClientRect().height)
);

if (view.width() !== width || view.height() !== height) {
view.width(width).height(height);
Expand All @@ -305,7 +322,7 @@ export class VegaBaseView {
if (this._parser.tooltips) {
// position and padding can be specified with
// {config:{kibana:{tooltips: {position: 'top', padding: 15 } }}}
const tthandler = new TooltipHandler(this._$container[0], view, this._parser.tooltips);
const tthandler = new TooltipHandler(this._container, view, this._parser.tooltips);

// Vega bug workaround - need to destroy tooltip by hand
this._addDestroyHandler(() => tthandler.hideTooltip());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ describe('vega_map_view/view', () => {
layers: [],
},
customAttribution: 'tilemap-attribution',
container: vegaMapView._$container.get(0),
container: vegaMapView._container,
minZoom: 0,
maxZoom: 20,
zoom: 3,
Expand All @@ -200,7 +200,7 @@ describe('vega_map_view/view', () => {
layers: [],
},
customAttribution: ['<a rel="noreferrer noopener" href="tms_attributions"></a>'],
container: vegaMapView._$container.get(0),
container: vegaMapView._container,
minZoom: 0,
maxZoom: 20,
zoom: 3,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,14 @@ export class VegaMapView extends VegaBaseView {
}

// In some cases, Vega may be initialized twice, e.g. after awaiting...
if (!this._$container) return;
if (!this._container) return;

// For the correct geration of the PDF/PNG report, we must wait until the map is fully rendered.
return new Promise((resolve) => {
const mapBoxInstance = new maplibregl.Map({
style,
customAttribution,
container: this._$container.get(0),
container: this._container,
...this.getMapParams({ ...zoomSettings }),
});

Expand Down Expand Up @@ -206,7 +206,7 @@ export class VegaMapView extends VegaBaseView {
map: mapBoxInstance,
context: {
vegaView,
vegaControls: this._$controls?.get(0),
vegaControls: this._controls,
updateVegaView,
},
});
Expand Down
4 changes: 2 additions & 2 deletions src/plugins/vis_types/vega/public/vega_view/vega_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ import { VegaBaseView } from './vega_base_view';
export class VegaView extends VegaBaseView {
async _initViewCustomizations() {
// In some cases, Vega may be initialized twice... TBD
if (!this._$container) return;
if (!this._container) return;

const view = new View(parse(this._parser.spec, undefined, { ast: true }), this._vegaViewConfig);

if (this._parser.useResize) this.updateVegaSize(view);
view.initialize(this._$container.get(0), this._$controls.get(0));
view.initialize(this._container, this._controls);
// resize again to take controls into account
if (this._parser.useResize) this.updateVegaSize(view);

Expand Down
13 changes: 5 additions & 8 deletions src/plugins/vis_types/vega/public/vega_visualization.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

import 'jest-canvas-mock';

import $ from 'jquery';

import { createVegaVisualization } from './vega_visualization';

import vegaliteGraph from './test_utils/vegalite_graph.json';
Expand All @@ -32,9 +30,8 @@ describe('VegaVisualizations', () => {
let VegaVisualization;
let vegaVisualizationDependencies;

let mockWidth;
let mockGetBoundingClientRect;
let mockedWidthValue;
let mockHeight;
let mockedHeightValue;

const coreStart = coreMock.createStart();
Expand All @@ -46,8 +43,9 @@ describe('VegaVisualizations', () => {
mockedHeightValue = height;
domNode = document.createElement('div');

mockWidth = jest.spyOn($.prototype, 'width').mockImplementation(() => mockedWidthValue);
mockHeight = jest.spyOn($.prototype, 'height').mockImplementation(() => mockedHeightValue);
mockGetBoundingClientRect = jest
.spyOn(Element.prototype, 'getBoundingClientRect')
.mockImplementation(() => ({ width: mockedWidthValue, height: mockedHeightValue }));
};

const mockGetServiceSettings = () => {
Expand Down Expand Up @@ -78,8 +76,7 @@ describe('VegaVisualizations', () => {
});

afterEach(() => {
mockWidth.mockRestore();
mockHeight.mockRestore();
mockGetBoundingClientRect.mockRestore();
});

test('should show vegalite graph and update on resize (may fail in dev env)', async () => {
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/canvas/public/application.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { i18n } from '@kbn/i18n';
import { Provider } from 'react-redux';
import { BehaviorSubject } from 'rxjs';

import '@kbn/flot-charts';
import { includes, remove } from 'lodash';

import { AppMountParameters, CoreStart, CoreSetup, AppUpdater } from '@kbn/core/public';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
* 2.0.
*/

import $ from 'jquery';

// Kibana wrapper
import d3 from 'd3';
import { getIcon } from '../../helpers/style_choices';
Expand All @@ -15,31 +13,25 @@ import { getIcon } from '../../helpers/style_choices';
// for use outside of Kibana server with direct access to elasticsearch
let graphExplorer = function (indexName, typeName, request, responseHandler) {
const dataForServer = JSON.stringify(request);
$.ajax({
type: 'POST',
url: 'http://localhost:9200/' + indexName + '/_graph/explore',
fetch(`http://localhost:9200/${indexName}/_graph/explore`, {
method: 'POST',
dataType: 'json',
contentType: 'application/json;charset=utf-8',
async: true,
data: dataForServer,
success: function (data) {
responseHandler(data);
},
});
})
.then((response) => response.json())
.then(responseHandler);
};
let searcher = function (indexName, request, responseHandler) {
const dataForServer = JSON.stringify(request);
$.ajax({
type: 'POST',
url: 'http://localhost:9200/' + indexName + '/_search?rest_total_hits_as_int=true',
fetch(`http://localhost:9200/${indexName}/_search?rest_total_hits_as_int=true`, {
method: 'POST',
dataType: 'json',
contentType: 'application/json;charset=utf-8', //Not sure why this was necessary - worked without elsewhere
async: true,
contentType: 'application/json;charset=utf-8',
data: dataForServer,
success: function (data) {
responseHandler(data);
},
});
})
.then((response) => response.json())
.then(responseHandler);
};

// ====== Undo operations =============
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/monitoring/public/components/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
* 2.0.
*/

/** Import this dependency here to load it async rather than on startup */
import '@kbn/flot-charts';
/*
* This file should only export page-level components for view controllers to
* mount React to the DOM
*/

export { NoData } from './no_data';
export { License } from './license';
export { PageLoading } from './page_loading';
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/monitoring/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"@kbn/rule-data-utils",
"@kbn/react-kibana-mount",
"@kbn/react-kibana-context-render",
"@kbn/flot-charts",
],
"exclude": [
"target/**/*",
Expand Down

0 comments on commit ac5ace0

Please sign in to comment.