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

TypeScriptify visualization components #20940

Merged
merged 14 commits into from
Jul 23, 2018
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,4 @@
* under the License.
*/

export * from './visualization';
export * from './visualization_chart';
export * from './visualization_noresults';
export { PersistedState } from './persisted_state';
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,6 @@
* under the License.
*/

import React from 'react';

export function VisualizationNoResults() {
return (
<div className="text-center visualize-error visualize-chart">
<div className="item top" />
<div className="item">
<h2 aria-hidden="true"><i aria-hidden="true" className="fa fa-meh-o" /></h2>
<h4>No results found</h4>
</div>
<div className="item bottom" />
</div>
);
}
// It's currenty hard to properly type PersistedState, since it dynamically
// inherits the class passed into the constructor.
export type PersistedState = any;
50 changes: 50 additions & 0 deletions src/ui/public/utils/memoize.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { memoizeLast } from './memoize';

describe('memoizeLast', () => {
type SumFn = (a: number, b: number) => number;
let originalSum: SumFn;
let memoizedSum: SumFn;

beforeEach(() => {
originalSum = jest.fn((a, b) => a + b);
memoizedSum = memoizeLast(originalSum);
});

it('should call through function', () => {
expect(memoizedSum(26, 16)).toBe(42);
expect(originalSum).toHaveBeenCalledWith(26, 16);
});

it('should memoize the last call', () => {
memoizedSum(26, 16);
expect(originalSum).toHaveBeenCalledTimes(1);
memoizedSum(26, 16);
expect(originalSum).toHaveBeenCalledTimes(1);
});

it('should use parameters as cache keys', () => {
expect(memoizedSum(26, 16)).toBe(42);
expect(originalSum).toHaveBeenCalledTimes(1);
expect(memoizedSum(16, 26)).toBe(42);
expect(originalSum).toHaveBeenCalledTimes(2);
});
});
51 changes: 51 additions & 0 deletions src/ui/public/utils/memoize.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

/**
* A simple memoize function, that only stores the last returned value
* and uses the identity of all passed parameters as a cache key.
*/
function memoizeLast<T extends (...args: any[]) => any>(func: T): T {
let prevResult: any;
let called: boolean = false;
Copy link
Member

Choose a reason for hiding this comment

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

We could get rid of called by initializing prevArgs to an array containing a unique symbol that is not exported from this module. Then the conditional inside memoizedFunction would never be true on the first execution.

let prevThis: any;
let prevArgs: any[];

// We need to use a `function` here for proper this passing.
// tslint:disable-next-line:only-arrow-functions
const memoizedFunction = function(this: any, ...args: any[]) {
if (
called &&
prevThis === this &&
prevArgs.length === args.length &&
args.every((arg, index) => arg === prevArgs[index])
) {
return prevResult;
}
called = true;
prevThis = this;
prevArgs = args;
prevResult = func.apply(this, args);
return prevResult;
} as T;

return memoizedFunction;
}

export { memoizeLast };
1 change: 1 addition & 0 deletions src/ui/public/vis/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@

export { AggConfig } from './agg_config';
export { Vis, VisProvider } from './vis';
export { VisualizationController, VisType } from './vis_types/vis_type';
3 changes: 2 additions & 1 deletion src/ui/public/vis/update_status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/

import { PersistedState } from '../persisted_state';
import { calculateObjectHash } from './lib/calculate_object_hash';
import { Vis } from './vis';

Expand Down Expand Up @@ -57,7 +58,7 @@ function hasSizeChanged(size: Size, oldSize?: Size): boolean {
function getUpdateStatus<T extends Status>(
requiresUpdateStatus: T[] = [],
obj: any,
param: { vis: Vis; visData: any; uiState: any }
param: { vis: Vis; visData: any; uiState: PersistedState }
): { [reqStats in T]: boolean } {
const status = {} as { [reqStats in T]: boolean };

Expand Down
12 changes: 11 additions & 1 deletion src/ui/public/vis/vis.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@
* under the License.
*/

export type Vis = any;
import { VisType } from './vis_types/vis_type';

export interface Vis {
type: VisType;

// Since we haven't typed everything here yet, we basically "any" the rest
// of that interface. This should be removed as soon as this type definition
// has been completed. But that way we at least have typing for a couple of
// properties on that type.
[key: string]: any;
}

export type VisProvider = (...dependencies: any[]) => Vis;
40 changes: 40 additions & 0 deletions src/ui/public/vis/vis_types/vis_type.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { Vis } from '..';
import { Status } from '../update_status';

export class VisualizationController {
constructor(element: HTMLElement, vis: Vis);
public render(visData: any, update: { [key in Status]: boolean }): Promise<void>;
Copy link
Member

Choose a reason for hiding this comment

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

Given that { [key in Status]: boolean } is used in more than one place now (e.g. here and the return value of getUpdateStatus), how about factoring it out into its own type in update_status.ts?

export type StatusUpdates<Keys extends Status> = {
  [key in Keys]: boolean;
}

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 think that's actually not working that well, since in the getUpdateStatus we actually use a different index signature, since we actually make sure, it returns an object only containing the status updates, that have been request as parameter. So I currently don't see any other place we would be using this - and hopefully it won't become more, because we are trying to get rid of the status calculation and not increasing it :D

Copy link
Member

Choose a reason for hiding this comment

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

good point 👍

public destroy(): void;
public isLoaded?(): Promise<void> | void;
}

export interface VisType {
title: string;
visualization: typeof VisualizationController;
isAccessible?: boolean;

// Since we haven't typed everything here yet, we basically "any" the rest
// of that interface. This should be removed as soon as this type definition
// has been completed. But that way we at least have typing for a couple of
// properties on that type.
[key: string]: any;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`VisualizationNoResults should render according to snapshot 1`] = `
<div
class="text-center visualize-error visualize-chart"
>
<div
class="item top"
/>
<div
class="item"
>
<h2
aria-hidden="true"
>
<i
aria-hidden="true"
class="fa fa-meh-o"
/>
</h2>
<h4>
No results found
</h4>
</div>
<div
class="item bottom"
/>
</div>
`;
20 changes: 20 additions & 0 deletions src/ui/public/visualize/components/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export { Visualization } from './visualization';
87 changes: 0 additions & 87 deletions src/ui/public/visualize/components/visualization.js

This file was deleted.

Loading