Skip to content
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
14 changes: 10 additions & 4 deletions frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"license": "Apache-2.0",
"private": true,
"workspaces": [
"packages/*"
"packages/*",
"public"
],
"scripts": {
"dev": "rm -rf ./public/dist && ts-node -O '{\"module\":\"commonjs\"}' ./node_modules/.bin/webpack --mode development --watch --progress",
Expand Down Expand Up @@ -36,7 +37,7 @@
".(ts|tsx|js|jsx)": "./node_modules/ts-jest/preprocessor.js"
},
"transformIgnorePatterns": [
"<rootDir>/node_modules/(?!lodash-es/.*)"
"<rootDir>/node_modules/(?!lodash-es|@console)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify, this change tells Jest to transform (basically "un-ignore") any @console module paths, in addition to existing lodash-es module paths.

More specifically, without this change, following module dependency chain:

__tests__/components/resource-pages.spec.tsx
public/components/resource-pages.ts
public/plugins.ts

will throw syntax errors (export * from statements) since public/plugins.ts imports from @console/plugin-sdk and Jest doesn't transform stuff in node_modules by default.

This is also the reason why all Console monorepo packages should use the @console scope in their name.

],
"testRegex": "/__tests__/.*\\.(ts|tsx|js|jsx)$",
"setupFiles": [
Expand Down Expand Up @@ -73,6 +74,7 @@
"js-base64": "^2.4.5",
"js-yaml": "3.x",
"lodash-es": "4.x",
"memoize-one": "5.x",
"murmurhash-js": "1.0.x",
"openshift-logos-icon": "1.7.1",
"patternfly": "^3.59.0",
Expand Down Expand Up @@ -108,6 +110,7 @@
},
"devDependencies": {
"@types/enzyme": "3.x",
"@types/glob": "7.x",
"@types/immutable": "3.x",
"@types/jasmine": "2.8.x",
"@types/jasminewd2": "2.0.x",
Expand All @@ -121,7 +124,7 @@
"@types/react-router-dom": "4.2.7",
"@types/react-transition-group": "2.x",
"@types/react-virtualized": "9.x",
"@types/webpack": "^4.1.1",
"@types/webpack": "4.x",
"@typescript-eslint/eslint-plugin": "^1.7.0",
"@typescript-eslint/parser": "^1.7.0",
"bootstrap-sass": "^3.3.7",
Expand All @@ -137,6 +140,7 @@
"eslint-plugin-react-hooks": "^1.5.1",
"file-loader": "1.x",
"fork-ts-checker-webpack-plugin": "0.x",
"glob": "7.x",
"glslify-loader": "1.x",
"html-webpack-plugin": "3.x",
"jasmine-console-reporter": "2.x",
Expand All @@ -149,6 +153,7 @@
"protractor": "5.4.x",
"protractor-fail-fast": "3.x",
"protractor-jasmine2-screenshot-reporter": "0.5.x",
"read-pkg": "5.x",
"resolve-url-loader": "2.x",
"sass-loader": "6.x",
"thread-loader": "1.x",
Expand All @@ -158,7 +163,8 @@
"typescript": "3.4.4",
"webpack": "4.29.6",
"webpack-bundle-analyzer": "2.x",
"webpack-cli": "^2.0.12"
"webpack-cli": "^2.0.12",
"webpack-virtual-modules": "^0.1.10"
},
"engines": {
"node": ">=8.x"
Expand Down
2 changes: 2 additions & 0 deletions frontend/packages/console-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
"private": true,
"main": "src/index.ts",
"dependencies": {
"@console/internal": "0.0.0-fixed",
"@console/plugin-sdk": "0.0.0-fixed",
"@console/shared": "0.0.0-fixed"
}
}
2 changes: 1 addition & 1 deletion frontend/packages/console-app/src/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1 @@
import '../../../public/components/app';
import '@console/internal/components/app';
13 changes: 13 additions & 0 deletions frontend/packages/console-demo-plugin/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@console/demo-plugin",
"version": "0.0.0-fixed",
"description": "Demo plugin for Console web application",
"private": true,
"dependencies": {
"@console/plugin-sdk": "0.0.0-fixed",
"@console/shared": "0.0.0-fixed"
},
"consolePlugin": {
"entry": "src/plugin.ts"
}
}
55 changes: 55 additions & 0 deletions frontend/packages/console-demo-plugin/src/plugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import {
Plugin,
HrefNavItem,
ResourceNSNavItem,
ResourceListPage,
ResourceDetailPage,
} from '@console/plugin-sdk';

// TODO(vojtech): internal code needed by plugins should be moved to console-shared package
import { PodModel } from '@console/internal/models';

type ConsumedExtensions =
| HrefNavItem
| ResourceNSNavItem
| ResourceListPage
| ResourceDetailPage;

const plugin: Plugin<ConsumedExtensions> = [
{
type: 'NavItem/Href',
properties: {
section: 'Home',
componentProps: {
name: 'Test Href Link',
href: '/test',
},
},
},
{
type: 'NavItem/ResourceNS',
properties: {
section: 'Workloads',
componentProps: {
name: 'Test ResourceNS Link',
resource: 'pods',
},
},
},
{
type: 'ResourcePage/List',
properties: {
model: PodModel,
loader: () => import('@console/internal/components/pod' /* webpackChunkName: "pod" */).then(m => m.PodsPage),
},
},
{
type: 'ResourcePage/Detail',
properties: {
model: PodModel,
loader: () => import('@console/internal/components/pod' /* webpackChunkName: "pod" */).then(m => m.PodsDetailsPage),
},
},
];

export default plugin;
13 changes: 13 additions & 0 deletions frontend/packages/console-plugin-sdk/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@console/plugin-sdk",
"version": "0.0.0-fixed",
"description": "Console static plugin SDK",
"private": true,
"main": "src/index.ts",
"scripts": {
"test": "yarn --cwd ../.. run test packages/console-plugin-sdk"
Copy link
Contributor

Choose a reason for hiding this comment

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

What about e2e (integration tests), where do they fit?
Will we still collocate them on a single place considering they test whole application? Or split them per plugin?

The first option sounds better to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about e2e (integration tests), where do they fit?
Will we still collocate them on a single place considering they test whole application?

Yes, I think that's the general consensus. Unit tests, on the other hand, should be co-located with their corresponding package.

},
"dependencies": {
"@console/shared": "0.0.0-fixed"
}
}
61 changes: 61 additions & 0 deletions frontend/packages/console-plugin-sdk/src/codegen/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* eslint-env node */

import * as path from 'path';
import * as readPkg from 'read-pkg';

type Package = readPkg.NormalizedPackageJson;

interface PluginPackage extends Package {
consolePlugin: {
entry: string;
}
}

function isValidPluginPackage(pkg: Package): pkg is PluginPackage {
Copy link
Member

Choose a reason for hiding this comment

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

nit: we typically use function expressions in console:

Suggested change
function isValidPluginPackage(pkg: Package): pkg is PluginPackage {
const isValidPluginPackage = (pkg: Package): pkg is PluginPackage => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll keep that in mind for my future changes.

Personally, I find the typical function declaration more readable:

function name(args): returnType { body }

vs.

const name = (args): returnType => { body }

anyway, I'll try to stick with existing conventions used in Console code.

return (pkg as PluginPackage).consolePlugin && typeof (pkg as PluginPackage).consolePlugin.entry === 'string';
}

function readPackages(packageFiles: string[]) {
const pkgList: Package[] = packageFiles.map(file => readPkg.sync({ cwd: path.dirname(file), normalize: true }));

return {
appPackage: pkgList.find(pkg => pkg.name === '@console/app'),
pluginPackages: pkgList.filter(isValidPluginPackage),
};
}

/**
* Generate the "active plugins" module source.
*
* @param packageFiles Paths to `package.json` files (all the monorepo packages).
*/
export function getActivePluginsModule(packageFiles: string[]): string {
const { appPackage, pluginPackages } = readPackages(packageFiles);
let output = `
const activePlugins = [];
`;

if (appPackage) {
for (const depName of Object.keys(appPackage.dependencies)) {
const depVersion = appPackage.dependencies[depName];
const foundPluginPackage = pluginPackages.find(pkg => pkg.name === depName && pkg.version === depVersion);

if (foundPluginPackage) {
const importName = `plugin_${pluginPackages.indexOf(foundPluginPackage)}`;
const importPath = `${foundPluginPackage.name}/${foundPluginPackage.consolePlugin.entry}`;
output = `
${output}
import ${importName} from '${importPath}';
activePlugins.push(${importName});
`;
}
}
}

output = `
${output}
export default activePlugins;
`;

return output.replace(/^\s+/gm, '');
}
2 changes: 2 additions & 0 deletions frontend/packages/console-plugin-sdk/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './typings';
export * from './registry';
23 changes: 23 additions & 0 deletions frontend/packages/console-plugin-sdk/src/registry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import * as _ from 'lodash-es';
import { Extension, PluginList, isNavItem, isResourcePage } from './typings';

/**
* Registry used to query for Console extensions.
*/
export class ExtensionRegistry {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking forward to seeing this removed in favor of a HOC.


private readonly extensions: Extension<any>[];

public constructor(plugins: PluginList) {
this.extensions = _.flatMap(plugins);
}

public getNavItems(section: string) {
return this.extensions.filter(isNavItem).filter(e => e.properties.section === section);
}

public getResourcePages() {
return this.extensions.filter(isResourcePage);
}

}
66 changes: 66 additions & 0 deletions frontend/packages/console-plugin-sdk/src/typings/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* An extension of the Console web application.
*
* Each extension is a realization (instance) of an extension `type` using the
* parameters provided via the `properties` object.
*
* Core extension types should follow `Category` or `Category/Specialization`
* format, e.g. `NavItem/Href`.
*
* @todo(vojtech) write ESLint rule to guard against extension type duplicity
*/
export interface Extension<P> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use type here to discourage using classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In TypeScript, type aliases provide a name to refer to any kind of type, while interfaces are used to name object specific types.

Interface types have many similarities to type aliases for object type literals, but since interface types offer more capabilities they are generally preferred to type aliases.

An interface can be named in an extends or implements clause, but a type alias for an object type literal cannot.

An interface can have multiple merged declarations, but a type alias for an object type literal cannot.

When describing object types, I'd stick to the TS spec recommendation and prefer using interfaces over type aliases. Using interfaces does not imply using classes to implement those interfaces.

Copy link
Contributor

Choose a reason for hiding this comment

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

An interface can be named in an extends or implements clause, but a type alias for an object type literal cannot.

Not true. You can still implement a type; unless the type definition uses a union operator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

type TestObj = {
  x: number;
  y: () => boolean;
};

// valid
class Foo implements TestObj {
  x = 1;
  y() { return true; }
}

@christianvogt You are correct - you can use object literal type alias (e.g. TestObj) in the implements clause. It's also possible to do object composition via the intersection (&) operator:

type AnotherTestObj = TestObj & {
  z: string;
};

However, the spec is still correct in that you can't use e.g. TestObj in the extends clause, more specifically when declaring a class:

// valid
type Bar = <T extends TestObj>(obj: T) => void;

// invalid
class Qux extends TestObj {}

Still, I'd prefer using interfaces, as they describe the shape of an object (they're not a traditional OOP interface whose usage is coupled with classes).

type: string;
properties: P;
}

/**
* A plugin is simply a list of extensions.
*
* Plugin metadata is stored in the `package.json` file of the corresponding
* monorepo package. The `consolePlugin.entry` path should point to a module
* that exports the plugin object.
*
* ```json
* {
* "name": "@console/demo-plugin",
* "version": "0.0.0-fixed",
* // scripts, dependencies, etc.
* "consolePlugin": {
* "entry": "src/plugin.ts"
* }
* }
* ```
*
* For better type checking and code completion, use a type parameter that
* represents the union of all the extension types consumed by the plugin:
*
* ```ts
* // packages/console-demo-plugin/src/plugin.ts
* import { Plugin } from '@console/plugin-sdk';
*
* const plugin: Plugin<FooExtension | BarExtension> = [
* {
* type: 'Foo',
* properties: {} // Foo extension specific properties
* },
* {
* type: 'Bar',
* properties: {} // Bar extension specific properties
* }
* ];
*
* export default plugin;
* ```
*/
export type Plugin<E extends Extension<any>> = E[];

/**
* A list of arbitrary plugins.
*/
export type PluginList = Plugin<Extension<any>>[];

// TODO(vojtech): internal code needed by plugin SDK should be moved to console-shared package

export * from './nav';
export * from './pages';
50 changes: 50 additions & 0 deletions frontend/packages/console-plugin-sdk/src/typings/nav.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { Extension } from '.';
import { K8sKind } from '@console/internal/module/k8s';

export interface NavItemProperties {
// TODO(vojtech): link to existing nav sections by value
section: 'Home' | 'Workloads';
Copy link

Choose a reason for hiding this comment

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

These need to get updated to include all current sections

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, there's a TODO item for that.

I'd like to define some section title type (union of strings, like above) and use it in both NavSection and here.

componentProps: {
name: string;
required?: string;
disallowed?: string;
startsWith?: string[];
}
}

export interface HrefProperties extends NavItemProperties {
componentProps: NavItemProperties['componentProps'] & {
href: string;
activePath?: string;
}
}

export interface ResourceNSProperties extends NavItemProperties {
componentProps: NavItemProperties['componentProps'] & {
resource: string;
model?: K8sKind;
}
}

export interface HrefNavItem extends Extension<HrefProperties> {
type: 'NavItem/Href';
}

export interface ResourceNSNavItem extends Extension<ResourceNSProperties> {
type: 'NavItem/ResourceNS';
}

// TODO(vojtech): add ResourceClusterNavItem
export type NavItem = HrefNavItem | ResourceNSNavItem;

export function isHrefNavItem(e: Extension<any>): e is HrefNavItem {
return e.type === 'NavItem/Href';
}

export function isResourceNSNavItem(e: Extension<any>): e is ResourceNSNavItem {
return e.type === 'NavItem/ResourceNS';
}

export function isNavItem(e: Extension<any>): e is NavItem {
return isHrefNavItem(e) || isResourceNSNavItem(e);
}
Loading