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

Small Plugin Cleanup #206

Merged
merged 13 commits into from
Dec 9, 2021
2 changes: 1 addition & 1 deletion .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- name: install nodejs
uses: actions/setup-node@v2
with:
node-version: '16'
node-version-file: "ui/.nvmrc"
- uses: actions/cache@v2
with:
path: ~/.npm
Expand Down
32 changes: 16 additions & 16 deletions .github/workflows/react.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ jobs:
name: lint
runs-on: ubuntu-latest
steps:
- name: checkout
uses: actions/checkout@v2
- name: install nodejs
uses: actions/setup-node@v2
with:
node-version: '16'
- uses: actions/cache@v2
with:
path: ~/.npm
key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ runner.os }}-node-
- name: install deps
run: cd ./ui && npm ci
- run: cd ./ui && npm run lint
- name: checkout
uses: actions/checkout@v2
- name: install nodejs
uses: actions/setup-node@v2
with:
node-version-file: "ui/.nvmrc"
- uses: actions/cache@v2
with:
path: ~/.npm
key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ runner.os }}-node-
- name: install deps
run: cd ./ui && npm ci
- run: cd ./ui && npm run lint
test:
name: "unit test"
runs-on: ubuntu-latest
Expand All @@ -33,7 +33,7 @@ jobs:
- name: install nodejs
uses: actions/setup-node@v2
with:
node-version: '16'
node-version-file: "ui/.nvmrc"
- uses: actions/cache@v2
with:
path: ~/.npm
Expand Down
14 changes: 14 additions & 0 deletions dev/data/globaldatasource.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,20 @@
}
}
},
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just adding a browser data source to the sample data here...

"kind": "GlobalDatasource",
"metadata": {
"name": "PrometheusDemoBrowser"
},
"spec": {
"kind": "Prometheus",
"default": false,
"http": {
"access": "browser",
"url": "https://prometheus.demo.do.prometheus.io"
}
}
},
{
"kind": "GlobalDatasource",
"metadata": {
Expand Down
1 change: 1 addition & 0 deletions ui/.nvmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
v16.13.1
17 changes: 17 additions & 0 deletions ui/app/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright 2021 The Perses Authors
// Licensed 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 shared from '../jest.shared';

// Just use shared config as-is for now
export default shared;
10 changes: 0 additions & 10 deletions ui/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,5 @@
"webpack-cli": "^4.9.1",
"webpack-dev-server": "^4.6.0",
"webpack-merge": "^5.8.0"
},
"jest": {
"preset": "ts-jest",
"testRegex": "(/__tests__/.*|\\.(test|spec))\\.(ts|tsx|js|jsx)$",
"moduleFileExtensions": [
"ts",
"tsx",
"js",
"jsx"
]
}
}
26 changes: 13 additions & 13 deletions ui/app/src/context/plugin-registry/create-plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ import {
* Take a Variable plugin and wrap it so it works with AnyVariableDefinition,
* doing runtime checking of the definition before delegating to the plugin.
*/
export function createVariablePlugin<Kind extends string, Options extends JsonObject>(
config: PluginConfig<'Variable', Kind, Options>
export function createVariablePlugin<Options extends JsonObject>(
config: PluginConfig<'Variable', Options>
): AnyPluginImplementation<'Variable'> {
// Create runtime validation function
const useRuntimeValidation = createValidationHook(config);
Expand All @@ -53,14 +53,14 @@ export function createVariablePlugin<Kind extends string, Options extends JsonOb
* Take a Panel plugin and wraps it so it works with AnyPanelDefinition, doing
* runtime checking of the definition before delegating to the plugin.
*/
export function createPanelPlugin<Kind extends string, Options extends JsonObject>(
config: PluginConfig<'Panel', Kind, Options>
export function createPanelPlugin<Options extends JsonObject>(
config: PluginConfig<'Panel', Options>
): AnyPluginImplementation<'Panel'> {
const useRuntimeValidation = createValidationHook(config);

// Wrap PanelComponent from config with validation (TODO: Can this wrapper
// become generic for all Plugin components?)
function PanelComponent(props: PanelProps<string, JsonObject>) {
function PanelComponent(props: PanelProps<JsonObject>) {
const { definition, ...others } = props;

const { isValid, errorRef } = useRuntimeValidation();
Expand All @@ -80,8 +80,8 @@ export function createPanelPlugin<Kind extends string, Options extends JsonObjec
* Take a GraphQuery plugin and wrap it so it works with AnyChartQueryDefinition,
* doing runtime validation of the definition before delegating to the plugin.
*/
export function createGraphQueryPlugin<Kind extends string, Options extends JsonObject>(
config: PluginConfig<'GraphQuery', Kind, Options>
export function createGraphQueryPlugin<Options extends JsonObject>(
config: PluginConfig<'GraphQuery', Options>
): AnyPluginImplementation<'GraphQuery'> {
// Create runtime validation function
const useRuntimeValidation = createValidationHook(config);
Expand All @@ -102,23 +102,23 @@ export function createGraphQueryPlugin<Kind extends string, Options extends Json
}

// A hook for doing runtime validation of a PluginDefinition
type UseRuntimeValidationHook<Type extends PluginType, Kind extends string, Options extends JsonObject> = () => {
isValid: (definition: AnyPluginDefinition<Type>) => definition is PluginDefinition<Type, Kind, Options>;
type UseRuntimeValidationHook<Type extends PluginType, Options extends JsonObject> = () => {
isValid: (definition: AnyPluginDefinition<Type>) => definition is PluginDefinition<Type, Options>;
errorRef: React.MutableRefObject<InvalidPluginDefinitionError | undefined>;
};

// Create a hook for doing runtime validation of a plugin definition, given the
// plugin's config
function createValidationHook<Type extends PluginType, Kind extends string, Options extends JsonObject>(
config: PluginConfig<Type, Kind, Options>
): UseRuntimeValidationHook<Type, Kind, Options> {
function createValidationHook<Type extends PluginType, Options extends JsonObject>(
config: PluginConfig<Type, Options>
): UseRuntimeValidationHook<Type, Options> {
const useRuntimeValidation = () => {
// Ref for storing any validation errors as a side-effect of calling isValid
const errorRef = useRef<InvalidPluginDefinitionError | undefined>(undefined);

// Type guard that validates the generic runtime plugin definition data
// is correct for Kind/Options
const isValid = (definition: AnyPluginDefinition<Type>): definition is PluginDefinition<Type, Kind, Options> => {
const isValid = (definition: AnyPluginDefinition<Type>): definition is PluginDefinition<Type, Options> => {
// If they don't give us a validate function in the plugin config, not
// much we can do so just assume we're OK
const validateErrors = config.validate?.(definition) ?? [];
Expand Down
24 changes: 13 additions & 11 deletions ui/app/src/context/plugin-registry/registry-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
PluginType,
AnyPluginImplementation,
JsonObject,
ALL_PLUGIN_TYPES,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change to have a runtime list of all plugin types simplifies some of the registry code below so that now there are fewer places to go make changes whenever we add a new plugin type.

} from '@perses-ui/core';
import { BUNDLED_PLUGINS } from './bundled-plugins';
import { createGraphQueryPlugin, createPanelPlugin, createVariablePlugin } from './create-plugin';
Expand All @@ -43,11 +44,10 @@ export function useRegistryState(installedPlugins: PluginResource[]) {
// Go through all installed plugins and bundled plugins and build an index of
// those resources by type and kind
const loadablePlugins = useMemo(() => {
const loadableProps: PluginResourcesByTypeAndKind = {
Variable: new Map(),
Panel: new Map(),
GraphQuery: new Map(),
};
const loadableProps = {} as PluginResourcesByTypeAndKind;
for (const pluginType of ALL_PLUGIN_TYPES) {
loadableProps[pluginType] = new Map();
}

const addToLoadable = (resource: PluginResource) => {
const supportedKinds = resource.spec.supported_kinds;
Expand Down Expand Up @@ -75,15 +75,17 @@ export function useRegistryState(installedPlugins: PluginResource[]) {
return loadableProps;
}, [installedPlugins]);

const [plugins, setPlugins] = useImmer<LoadedPluginsByTypeAndKind>(() => ({
Variable: new Map(),
Panel: new Map(),
GraphQuery: new Map(),
}));
const [plugins, setPlugins] = useImmer<LoadedPluginsByTypeAndKind>(() => {
const loadedPlugins = {} as LoadedPluginsByTypeAndKind;
for (const pluginType of ALL_PLUGIN_TYPES) {
loadedPlugins[pluginType] = new Map();
}
return loadedPlugins;
});

// Create the register callback to pass to the module's setup function
const registerPlugin: RegisterPlugin = useCallback(
<Kind extends string, Options extends JsonObject>(config: PluginRegistrationConfig<Kind, Options>) => {
<Options extends JsonObject>(config: PluginRegistrationConfig<Options>) => {
switch (config.pluginType) {
case 'Variable':
setPlugins((draft) => {
Expand Down
2 changes: 1 addition & 1 deletion ui/app/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import App from './App';
import { createTheme } from './styles/theme';
import { SnackbarProvider } from './context/SnackbarProvider';

const queryClient = new QueryClient();
const queryClient = new QueryClient({ defaultOptions: { queries: { refetchOnWindowFocus: false } } });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This just turns off the default react-query behavior that was causing PromQL queries, etc. to be refetched anytime you focus the browser window.

Copy link
Member

Choose a reason for hiding this comment

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

cool ! That was simple at then. Nice catch !

Do you know what this behavior is the one by default ? It's a bit odd to have it by default, but probably I miss some context

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I felt like it was a weird choice to have by default too, so don't think you're missing anything. The docs have a bit of an explanation of some of the defaults:

https://react-query.tanstack.com/guides/important-defaults


function renderApp() {
ReactDOM.render(
Expand Down
17 changes: 6 additions & 11 deletions ui/app/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@
"compilerOptions": {
"sourceMap": true,
"outDir": "./dist",
"rootDir": "../",
"tsBuildInfoFile": "../.build-cache/app.tsbuildinfo",
"incremental": true
"incremental": true,
// Don't need composite since the app is an endpoint project
"composite": false,
"declaration": false,
"declarationMap": false
},
"references": [
{
Expand All @@ -21,13 +24,5 @@
"path": "../prometheus-plugin",
"prepend": false
}
],
// For webpack builds that use ts-node to compile the configs
"ts-node": {
"compilerOptions": {
"module": "commonjs",
"target": "es5",
"esModuleInterop": true
}
}
]
}
1 change: 1 addition & 0 deletions ui/app/webpack.common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export const commonConfig: Configuration = {
new ForkTsCheckerWebpackPlugin({
typescript: {
configFile: path.resolve(__dirname, './tsconfig.json'),
build: true, // Since we use project references...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should fix the error that we see sometimes locally where TypeScript complains that "XXX has not been built from source file YYY". I also tried to clean-up the tsconfig options a bit. So far seems to have worked 🤞

},
eslint: {
files: '../*/src/**/*.{ts,tsx,js,jsx}',
Expand Down
1 change: 1 addition & 0 deletions ui/app/webpack.dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const devConfig: Configuration = {
port: parseInt(process.env.PORT ?? '3000'),
open: true,
https: getHttpsConfig(),
http2: process.env.HTTP2 === 'true',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allows enabling HTTP2 for the webpack dev server (like I mentioned in chat).

historyApiFallback: true,
allowedHosts: 'all',
proxy: {
Expand Down
6 changes: 3 additions & 3 deletions ui/core/src/hooks/datasource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ export function useDataSources(selector: DatasourceSelector): GlobalDatasourceMo
return usePluginRuntime('useDataSources')(selector);
}

export function useDataSourceConfig<Kind extends string>(
export function useDataSourceConfig<T extends DatasourceSpecDefinition>(
selector: DatasourceSelector,
validate: (value: AnyDatasourceSpecDefinition) => value is DatasourceSpecDefinition<Kind>
): DatasourceSpecDefinition<Kind> {
validate: (value: AnyDatasourceSpecDefinition) => value is T
): T {
const datasources = useDataSources(selector);

if (datasources.length !== 1 || datasources[0] === undefined) {
Expand Down
2 changes: 1 addition & 1 deletion ui/core/src/hooks/graph-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ import { usePluginRuntime } from '../context/PluginRuntimeContext';
import { UseGraphQueryHook } from '../model/graph-query';
import { JsonObject } from '../model/definitions';

export const useGraphQuery: UseGraphQueryHook<string, JsonObject> = (definition) => {
export const useGraphQuery: UseGraphQueryHook<JsonObject> = (definition) => {
return usePluginRuntime('useGraphQuery')(definition);
};
7 changes: 4 additions & 3 deletions ui/core/src/model/datasource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@

import { Metadata, ProjectMetadata } from './resource';

export interface DatasourceSpecDefinition<Kind extends string> {
kind: Kind;
// TODO: Should use Definition<> and JsonObject?
export interface DatasourceSpecDefinition {
kind: string;
default: boolean;
}

export type AnyDatasourceSpecDefinition = DatasourceSpecDefinition<string>;
export type AnyDatasourceSpecDefinition = DatasourceSpecDefinition;

export interface DatasourceModel {
kind: 'Datasource';
Expand Down
4 changes: 2 additions & 2 deletions ui/core/src/model/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export type Json = string | number | boolean | null | JsonObject | Json[];
/**
* Base type for definitions in JSON config resources.
*/
export interface Definition<Kind extends string, Options extends JsonObject> extends JsonObject {
kind: Kind;
export interface Definition<Options extends JsonObject> extends JsonObject {
kind: string;
options: Options;
}
11 changes: 4 additions & 7 deletions ui/core/src/model/graph-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,15 @@ import { Definition, JsonObject } from './definitions';
import { AnyPluginDefinition, AnyPluginImplementation } from './plugins';
import { ResourceSelector } from './resource';

export interface GraphQueryDefinition<Kind extends string, Options extends JsonObject>
extends Definition<Kind, Options> {
export interface GraphQueryDefinition<Options extends JsonObject> extends Definition<Options> {
datasource?: ResourceSelector;
}

export interface GraphQueryPlugin<Kind extends string, Options extends JsonObject> {
useGraphQuery: UseGraphQueryHook<Kind, Options>;
export interface GraphQueryPlugin<Options extends JsonObject> {
useGraphQuery: UseGraphQueryHook<Options>;
}

export type UseGraphQueryHook<Kind extends string, Options extends JsonObject> = (
definition: GraphQueryDefinition<Kind, Options>
) => {
export type UseGraphQueryHook<Options extends JsonObject> = (definition: GraphQueryDefinition<Options>) => {
data?: GraphData;
loading: boolean;
error?: Error;
Expand Down
14 changes: 6 additions & 8 deletions ui/core/src/model/panels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import { Definition, JsonObject } from './definitions';
import { AnyPluginDefinition, AnyPluginImplementation } from './plugins';

export interface PanelDefinition<Kind extends string, Options extends JsonObject> extends Definition<Kind, Options> {
export interface PanelDefinition<Options extends JsonObject> extends Definition<Options> {
display: {
name: string;
};
Expand All @@ -23,16 +23,14 @@ export interface PanelDefinition<Kind extends string, Options extends JsonObject
/**
* Plugin the provides custom visualizations inside of a Panel.
*/
export interface PanelPlugin<Kind extends string, Options extends JsonObject> {
PanelComponent: PanelComponent<Kind, Options>;
export interface PanelPlugin<Options extends JsonObject> {
PanelComponent: PanelComponent<Options>;
}

export type PanelComponent<Kind extends string, Options extends JsonObject> = React.ComponentType<
PanelProps<Kind, Options>
>;
export type PanelComponent<Options extends JsonObject> = React.ComponentType<PanelProps<Options>>;

export interface PanelProps<Kind extends string, Options extends JsonObject> {
definition: PanelDefinition<Kind, Options>;
export interface PanelProps<Options extends JsonObject> {
definition: PanelDefinition<Options>;
}

export type AnyPanelDefinition = AnyPluginDefinition<'Panel'>;
Expand Down