Skip to content

Conversation

@vojtechszocs
Copy link
Contributor

Follow-up to #1871 (un-merged)

This PR adds the ability to override the list of active Console plugins.

Active plugins are used

  • for testing: packages/console-app/src/__tests__/plugin-test-utils.ts creates a test registry from all active plugins (for "what's tested" vs. "what's built" consistency)
  • for building: webpack.config.ts creates a virtual @console/active-plugins module that returns the list of all active plugins for use in Console (i.e. plugins to be included in the output bundle)
  • for reporting: plugin-stats.ts dynamically loads entry modules of all active plugins (executed as part of devel & prod builds)

By default, plugins must be listed in packages/console-app/package.json file under dependencies key in order to be considered active.

With this PR, it's possible to override that via CONSOLE_PLUGINS env. variable:

# use standard active plugin resolution logic
yarn dev

# for core Console development (no additional plugins)
CONSOLE_PLUGINS=@console/app yarn dev

# for customized plugin development experience
CONSOLE_PLUGINS=@console/app,@console/foo,@console/bar yarn dev

The CONSOLE_PLUGINS override is consistently applied to

  • dev (webpack)
  • build (webpack)
  • test (Jest / extension checks)
  • plugin-stats (ts-node)

/cc @spadgett @alecmerdler @christianvogt

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 1, 2019
@vojtechszocs
Copy link
Contributor Author

cc @rawagner @mareklibra @jelkosz

return appPackage ? pluginFilter(appPackage, pluginPackages) : [];

if (!appPackage) {
throw new Error(`Cannot detect Console application package ${consoleAppName}`);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this check was added to reflect the expectation that Console (app) itself is a plugin, i.e. packages/console-app/src/plugin.tsx.

@vojtechszocs
Copy link
Contributor Author

@alecmerdler @spadgett If you'd like, we can use a shorter form e.g.

CONSOLE_PLUGINS=app,foo,bar yarn dev

instead of

CONSOLE_PLUGINS=@console/app,@console/foo,@console/bar yarn dev

since all Console plugin pkg names are expected to use @console scope anyway.

@spadgett spadgett added this to the v4.2 milestone Jul 2, 2019
@christianvogt
Copy link
Contributor

@vojtechszocs being explicit is fine. You could add support for both and if there's no scope you assume the @console scope.

@vojtechszocs
Copy link
Contributor Author

@vojtechszocs being explicit is fine. You could add support for both and if there's no scope you assume the @console scope.

Agreed, will do.

@alecmerdler
Copy link
Contributor

Before:

$ yarn run dev
...
9525 modules
Time: 52448ms

After:

$ yarn run dev
...
8594 modules
Time: 44883ms

Nice!

Copy link
Contributor

@alecmerdler alecmerdler left a comment

Choose a reason for hiding this comment

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

Thanks for adding this!
/lgtm

(window as any).localStorage = (window as any).sessionStorage = {
setItem(key, value) {
return Object.assign(_localStorage, {[key]: value});
Object.assign(_localStorage, { [key]: value} );
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this does adhere to the real web API. But the existing way is what it should be (in a FP world).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, the web storage API could be better 😃 the above change just reflects the method signature in node_modules/typescript/lib/lib.dom.d.ts

setItem(key: string, value: string): void;

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 3, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alecmerdler, vojtechszocs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 3, 2019
@openshift-merge-robot openshift-merge-robot merged commit 2421cae into openshift:master Jul 4, 2019
@logonoff logonoff deleted the override-plugin-list branch September 25, 2025 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants