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: Re-rendering visualization when expression changes and improves typing #1491

Merged
merged 3 commits into from
Apr 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export const avatarFn = (): ExpressionFunctionDefinition<
fn: (input, args) => {
return {
type: 'render',
as: 'avatar',
as: 'avatar', // the expression renderer to use
value: {
name: args.name,
size: args.size,
Expand Down
1 change: 1 addition & 0 deletions examples/expressions_example/opensearch_dashboards.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"requiredPlugins": [
"navigation",
"expressions",
"visualizations",
"developerExamples",
"opensearchDashboardsReact"
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ import {
} from '@elastic/eui';
import { i18n } from '@osd/i18n';
import { FormattedMessage } from '@osd/i18n/react';
import React, { useEffect, useState } from 'react';
import React, { useEffect, useMemo, useState } from 'react';
import { ReactExpressionRenderer } from '../../../../src/plugins/expressions/public';
import { useOpenSearchDashboards } from '../../../../src/plugins/opensearch_dashboards_react/public';
import { PersistedState } from '../../../../src/plugins/visualizations/public';
import { ExpressionsExampleServices } from '../types';

interface Props {
Expand All @@ -43,6 +44,8 @@ export function PlaygroundSection({
const [input, setInput] = useState(defaultInput);
const [expression, setExpression] = useState(defaultExpression);
const [result, setResult] = useState('');
// Visualizations require the uiState to persist even when the expression changes
const uiState = useMemo(() => new PersistedState(), []);

useEffect(() => {
let isMounted = true;
Expand Down Expand Up @@ -126,7 +129,11 @@ export function PlaygroundSection({
iconType="help"
/>
<EuiSpacer />
<ReactExpressionRenderer expression={expression} className="playgroundRenderer" />
<ReactExpressionRenderer
expression={expression}
uiState={uiState}
className="playgroundRenderer"
/>
</>
) : (
<EuiCodeBlock>
Expand Down
26 changes: 10 additions & 16 deletions src/plugins/expressions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ string for you, as well as a series of registries for advanced users who might
want to incorporate their own functions, types, and renderers into the service
for use in their own application.

`Expressions` is a simple custom language designed to write a chain of functions that _pipe_ its output to the
input of the next function. When two or more such functions are chained together, it is an expressions pipeline. Since it is a custom language, any expression can be represented as a string. Functions can be configured using arguments provided. The final output of the expression pipeline can either be rendered using
one of the _renderers_ registered in `expressions` plugin or made to output the result of the final function in the chain.
`Expressions` is a simple custom language designed to write a chain of functions that _pipe_ its output to the input of the next function. When two or more such functions are chained together, it is an expressions pipeline. Since it is a custom language, any expression can be represented as a string. Functions can be configured using arguments provided. The final output of the expression pipeline can either be rendered using one of the _renderers_ registered in `expressions` plugin or made to output the result of the final function in the chain.

> It is not necessary to chain functions and a single function can be used in isolation.

Expand All @@ -17,8 +15,8 @@ Below is an example of an expression that renders a metric visualization that ag

```
opensearchDashboards
| opensearchaggs
index='d3d7af60-4c81-11e8-b3d7-01146121b73d'
| opensearchaggs
index='d3d7af60-4c81-11e8-b3d7-01146121b73d'
aggConfigs='[{"id":"1","type":"avg","params":{"field":"AvgTicketPrice","customLabel":"Avg. Ticket Price"}}]'
| metricVis
metric={visdimension accessor=0 format='number'}
Expand All @@ -34,7 +32,7 @@ Consider the example below where the expression performs the following. It takes
sleep time=2000 | square
```

**Note:** The above example expression functions are only available with the `--run-examples` flag
**Note:** The above example expression function is only available with the `--run-examples` flag

The whole string is an expression. `sleep` and `square` are expression functions registered with the expression plugin. `time=2000` is the argument passed to the `sleep` funciton with the value `2000`. `|` is used to denote pipe between the two functions. Every expression can take an input. In the example above, the input provided will be passed on by the sleep function to the square function.

Expand All @@ -49,31 +47,27 @@ const expression = `sleep time=2000 | square`;
const execution = expressions.execute(expression, input);
```

**Note:** The above example expression functions are only available with the `--run-examples` flag
**Note:** The above example expression function is only available with the `--run-examples` flag

### Rendering Expressions

The other way an expression can be used is to render an output using one of the _renderers_ registered in `expressions` plugin. This can be done using a few ways, the easiest of which is to use the `ReactExpressionRenderer` component.
The other way an expression can be used is to render an output using one of the _renderers_ registered in `expressions` plugin. This can be done using a few ways, the easiest of which is to use the `ReactExpressionRenderer` component.

```jsx
const expressionString = `avatar name="OpenSearch Dashboards" size="xl"`;
<ReactExpressionRenderer expression={expressionString} />
<ReactExpressionRenderer expression={expressionString} />;
```

**Note:** The above example expression functions are only available with the `--run-examples` flag
**Note:** The above example expression function is only available with the `--run-examples` flag

## Custom expressions

Users can extend the service to incorporate their own functions, types, and renderers. Examples of these can be found in `./examples/expressions_example/common/expression_functions` and can be registered using the `registerFunction`, `registertype` and `registerRenderer` api's from the expression setup contract.
Users can extend the service to incorporate their own functions, types, and renderers. Examples of these can be found in `./examples/expressions_example/common/expression_functions` and can be registered using the `registerFunction`, `registertype` and `registerRenderer` api's from the expression setup contract.

## Playground

Working with expressions can sometimes be a little tricky. To make this easier we have an example plugin with some examples, a playground to run your own expression functions and explorer to view all the registered expression functions and their propoerties. It can be started up using the `--run-examples` flag and found under the `Developer examples` option in the main menu.
Working with expressions can sometimes be a little tricky. To make this easier we have an example plugin with some examples, a playground to run your own expression functions and explorer to view all the registered expression functions and their properties. It can be started up using the `--run-examples` flag and found under the `Developer examples` option in the main menu.

```sh
yarn start --run-examples
```




Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,5 @@ export interface IInterpreterRenderHandlers {
reload: () => void;
update: (params: any) => void;
event: (event: any) => void;
uiState?: any;
Copy link
Member

Choose a reason for hiding this comment

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

nit - shouldn't this be PersistedState type?

Copy link
Member Author

Choose a reason for hiding this comment

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

It could. I wasn't a 100% if uiState was always used as PersistedState everywhere. It is used as PersistedState in the viz renderer but other renderers can use other uiState values. To not break anything I just kept the type that was expected everywhere else. I may update this once I can be sure that it won't break anything.

}
4 changes: 2 additions & 2 deletions src/plugins/expressions/public/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ export class ExpressionRenderHandler {
.render(this.element, data.value, {
...this.handlers,
uiState,
} as any);
});
} catch (e) {
return this.handleRenderError(e);
return this.handleRenderError(e as Error);
}
};

Expand Down
7 changes: 2 additions & 5 deletions src/plugins/input_control_vis/public/input_control_fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
*/

import { i18n } from '@osd/i18n';

import { VisRenderValue } from '../../visualizations/public';
import {
ExpressionFunctionDefinition,
OpenSearchDashboardsDatatable,
Expand All @@ -40,11 +40,8 @@ interface Arguments {
visConfig: string;
}

type VisParams = Required<Arguments>;

interface RenderValue {
interface RenderValue extends VisRenderValue {
visType: 'input_control_vis';
visConfig: VisParams;
}

export const createInputControlVisFn = (): ExpressionFunctionDefinition<
Expand Down
9 changes: 2 additions & 7 deletions src/plugins/vis_type_table/public/table_vis_fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,22 +35,17 @@ import {
OpenSearchDashboardsDatatable,
Render,
} from '../../expressions/public';
import { VisRenderValue } from '../../visualizations/public';

export type Input = OpenSearchDashboardsDatatable;

interface Arguments {
visConfig: string | null;
}

type VisParams = Required<Arguments>;

interface RenderValue {
interface RenderValue extends VisRenderValue {
visData: TableContext;
visType: 'table';
visConfig: VisParams;
params: {
listenOnChange: boolean;
};
}

export const createTableVisFn = (): ExpressionFunctionDefinition<
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/vis_type_timeseries/public/metrics_fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {

// @ts-ignore
import { metricsRequestHandler } from './request_handler';
import { VisRenderValue } from '../../visualizations/public';

type Input = OpenSearchDashboardsContext | null;
type Output = Promise<Render<RenderValue>>;
Expand All @@ -50,7 +51,7 @@ interface Arguments {

type VisParams = Required<Arguments>;

interface RenderValue {
interface RenderValue extends VisRenderValue {
visType: 'metrics';
visData: Input;
visConfig: VisParams;
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/vis_type_vega/public/vega_fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { VegaVisualizationDependencies } from './plugin';
import { createVegaRequestHandler } from './vega_request_handler';
import { VegaInspectorAdapters } from './vega_inspector/index';
import { TimeRange, Query } from '../../data/public';
import { VisRenderValue } from '../../visualizations/public';
import { VegaParser } from './data_model/vega_parser';

type Input = OpenSearchDashboardsContext | null;
Expand All @@ -51,7 +52,7 @@ interface Arguments {

export type VisParams = Required<Arguments>;

interface RenderValue {
interface RenderValue extends VisRenderValue {
visData: VegaParser;
visType: 'vega';
visConfig: VisParams;
Expand Down
3 changes: 2 additions & 1 deletion src/plugins/vis_type_vislib/public/pie_fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/

import { i18n } from '@osd/i18n';
import { VisRenderValue } from '../../visualizations/public';
import {
ExpressionFunctionDefinition,
OpenSearchDashboardsDatatable,
Expand All @@ -43,7 +44,7 @@ interface Arguments {

type VisParams = Required<Arguments>;

interface RenderValue {
interface RenderValue extends VisRenderValue {
visConfig: VisParams;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/

import { i18n } from '@osd/i18n';
import { VisRenderValue } from '../../visualizations/public';
import {
ExpressionFunctionDefinition,
OpenSearchDashboardsDatatable,
Expand All @@ -44,7 +45,7 @@ interface Arguments {

type VisParams = Required<Arguments>;

interface RenderValue {
interface RenderValue extends VisRenderValue {
visType: string;
visConfig: VisParams;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

import { get } from 'lodash';
import { i18n } from '@osd/i18n';
import { VisResponseValue, PersistedState } from '../../../../plugins/visualizations/public';
import { VisRenderValue, PersistedState } from '../';
import { ExpressionFunctionDefinition, Render } from '../../../../plugins/expressions/public';
import { getTypes, getIndexPatterns, getFilterManager, getSearch } from '../services';

Expand All @@ -49,7 +49,7 @@ export type ExpressionFunctionVisualization = ExpressionFunctionDefinition<
'visualization',
any,
Arguments,
Promise<Render<VisResponseValue>>
Promise<Render<VisRenderValue>>
>;

export const visualization = (): ExpressionFunctionVisualization => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,16 @@

import React from 'react';
import { render, unmountComponentAtNode } from 'react-dom';
// @ts-ignore
import { ExpressionRenderDefinition } from '../../../expressions/public';
import { ExprVis } from './vis';
import { Visualization } from '../components';
import { VisParams } from '../types';
import { VisParams, VisRenderValue } from '../types';

export const visualization = () => ({
export const visualization = (): ExpressionRenderDefinition<VisRenderValue> => ({
name: 'visualization',
displayName: 'visualization',
reuseDomNode: true,
render: async (domNode: HTMLElement, config: any, handlers: any) => {
render: async (domNode, config, handlers) => {
const { visData, visConfig, params } = config;
const visType = config.visType || visConfig.type;

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/visualizations/public/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export {
SavedVisState,
ISavedVis,
VisSavedObject,
VisResponseValue,
VisRenderValue,
} from './types';
export { ExprVisAPIEvents } from './expressions/vis';
export { VisualizationListItem } from './vis_types/vis_type_alias_registry';
Expand Down
15 changes: 11 additions & 4 deletions src/plugins/visualizations/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,18 @@ export interface ISavedVis {

export interface VisSavedObject extends SavedObject, ISavedVis {}

export interface VisResponseValue {
export interface VisRenderValue {
title?: string;
visType: string;
visData: object;
visConfig: object;
params?: object;
visData?: object | null;
Copy link
Member

Choose a reason for hiding this comment

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

nit - do we need to support null here? Generally undefined is probably preferable and covered by the optional.

Copy link
Member Author

Choose a reason for hiding this comment

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

One of the existing visualizations expects a null value for vizData when no data is present. Since I didn't want to introduce a breaking change, I kept the expected types the same and instead just typed the interfaces more strongly.

visConfig: {
type?: string;
[key: string]: any;
};
params?: {
[key: string]: any;
listenOnChange: boolean;
};
}

export interface VisToExpressionAstParams {
Expand Down