From c67230f32269af4d2d7c3a5b6cf605bae8ba1588 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Wed, 1 Mar 2023 07:46:08 -0800 Subject: [PATCH] Cleanup getDi and registering injectables (#7251) * Cleanup getDi and registering injectables Signed-off-by: Sebastian Malton * Create more explicit application - Make testing use more production code Signed-off-by: Sebastian Malton * Fix uses of getEnvironmentSpecificLegacyGlobalDiForExtensionApi Signed-off-by: Sebastian Malton * Fix unit tests Signed-off-by: Sebastian Malton --------- Signed-off-by: Sebastian Malton --- .../src/common/catalog-entities/web-link.ts | 4 +- packages/core/src/common/create-app.ts | 17 +++++++++ ...nformation-fake.injectable.testing-env.ts} | 0 .../vars/node-env.injectable.testing-env.ts | 14 +++++++ ...r-extension-api-with-modifications.test.ts | 4 +- .../legacy-global-di-for-extension-api.ts | 12 ++---- .../src/extensions/main-api/navigation.ts | 9 +---- packages/core/src/main/create-app.ts | 20 ++++------ packages/core/src/main/getDi.ts | 12 +++++- packages/core/src/main/getDiForUnitTesting.ts | 37 +++++------------- packages/core/src/main/library.ts | 3 +- .../core/src/main/register-injectables.ts | 8 ---- packages/core/src/renderer/create-app.ts | 19 +++++----- packages/core/src/renderer/getDi.tsx | 14 ++++++- .../core/src/renderer/getDiForUnitTesting.tsx | 38 +++++-------------- packages/core/src/renderer/library.ts | 3 +- .../core/src/renderer/register-injectables.ts | 11 ------ packages/open-lens/src/main/index.ts | 9 ++--- packages/open-lens/src/renderer/index.ts | 9 ++--- 19 files changed, 110 insertions(+), 133 deletions(-) create mode 100644 packages/core/src/common/create-app.ts rename packages/core/src/common/vars/{application-information-fake-injectable.ts => application-information-fake.injectable.testing-env.ts} (100%) create mode 100644 packages/core/src/common/vars/node-env.injectable.testing-env.ts diff --git a/packages/core/src/common/catalog-entities/web-link.ts b/packages/core/src/common/catalog-entities/web-link.ts index 7c83051c8b..833f05d65b 100644 --- a/packages/core/src/common/catalog-entities/web-link.ts +++ b/packages/core/src/common/catalog-entities/web-link.ts @@ -3,7 +3,7 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -import { Environments, getEnvironmentSpecificLegacyGlobalDiForExtensionApi } from "../../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; +import { getEnvironmentSpecificLegacyGlobalDiForExtensionApi } from "../../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; import type { CatalogEntityContextMenuContext, CatalogEntityMetadata, CatalogEntityStatus } from "../catalog"; import { CatalogCategory, CatalogEntity, categoryVersion } from "../catalog/catalog-entity"; import productNameInjectable from "../vars/product-name.injectable"; @@ -32,7 +32,7 @@ export class WebLink extends CatalogEntity Promise; + readonly di: DiContainerForInjection; +} + +export type CreateApplication = (config: ApplicationConfig) => Application; diff --git a/packages/core/src/common/vars/application-information-fake-injectable.ts b/packages/core/src/common/vars/application-information-fake.injectable.testing-env.ts similarity index 100% rename from packages/core/src/common/vars/application-information-fake-injectable.ts rename to packages/core/src/common/vars/application-information-fake.injectable.testing-env.ts diff --git a/packages/core/src/common/vars/node-env.injectable.testing-env.ts b/packages/core/src/common/vars/node-env.injectable.testing-env.ts new file mode 100644 index 0000000000..29231a4010 --- /dev/null +++ b/packages/core/src/common/vars/node-env.injectable.testing-env.ts @@ -0,0 +1,14 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import nodeEnvInjectionToken from "./node-env-injection-token"; + +const nodeEnvFakeInjectable = getInjectable({ + id: "node-env-fake", + instantiate: () => "production", + injectionToken: nodeEnvInjectionToken, +}); + +export default nodeEnvFakeInjectable; diff --git a/packages/core/src/extensions/as-legacy-globals-for-extension-api/as-legacy-global-object-for-extension-api-with-modifications.test.ts b/packages/core/src/extensions/as-legacy-globals-for-extension-api/as-legacy-global-object-for-extension-api-with-modifications.test.ts index ebfc77f40e..5c6c60f8e0 100644 --- a/packages/core/src/extensions/as-legacy-globals-for-extension-api/as-legacy-global-object-for-extension-api-with-modifications.test.ts +++ b/packages/core/src/extensions/as-legacy-globals-for-extension-api/as-legacy-global-object-for-extension-api-with-modifications.test.ts @@ -9,7 +9,7 @@ import { createContainer, getInjectable, } from "@ogre-tools/injectable"; -import { Environments, setLegacyGlobalDiForExtensionApi } from "./legacy-global-di-for-extension-api"; +import { setLegacyGlobalDiForExtensionApi } from "./legacy-global-di-for-extension-api"; import { asLegacyGlobalObjectForExtensionApiWithModifications } from "./as-legacy-global-object-for-extension-api-with-modifications"; describe("asLegacyGlobalObjectForExtensionApiWithModifications", () => { @@ -25,7 +25,7 @@ describe("asLegacyGlobalObjectForExtensionApiWithModifications", () => { jest.spyOn(di, "inject"); - setLegacyGlobalDiForExtensionApi(di, Environments.renderer); + setLegacyGlobalDiForExtensionApi(di, "renderer"); someInjectable = getInjectable({ id: "some-injectable", diff --git a/packages/core/src/extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api.ts b/packages/core/src/extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api.ts index 4d7d9ca8f5..59d3a5465c 100644 --- a/packages/core/src/extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api.ts +++ b/packages/core/src/extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api.ts @@ -4,17 +4,11 @@ */ import type { DiContainer } from "@ogre-tools/injectable"; +export type Environments = "main" | "renderer"; + const legacyGlobalDis = new Map(); -export enum Environments { - renderer, - main, -} - -export const setLegacyGlobalDiForExtensionApi = ( - di: DiContainer, - environment: Environments, -) => { +export const setLegacyGlobalDiForExtensionApi = (di: DiContainer, environment: Environments) => { legacyGlobalDis.set(environment, di); }; diff --git a/packages/core/src/extensions/main-api/navigation.ts b/packages/core/src/extensions/main-api/navigation.ts index 375d2bac74..2bd870a262 100644 --- a/packages/core/src/extensions/main-api/navigation.ts +++ b/packages/core/src/extensions/main-api/navigation.ts @@ -3,16 +3,11 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -import { - Environments, - getEnvironmentSpecificLegacyGlobalDiForExtensionApi, -} from "../as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; - +import { getEnvironmentSpecificLegacyGlobalDiForExtensionApi } from "../as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; import navigateInjectable from "../../main/start-main-application/lens-window/navigate.injectable"; export function navigate(url: string) { - const di = getEnvironmentSpecificLegacyGlobalDiForExtensionApi(Environments.main); - + const di = getEnvironmentSpecificLegacyGlobalDiForExtensionApi("main"); const navigate = di.inject(navigateInjectable); return navigate(url); diff --git a/packages/core/src/main/create-app.ts b/packages/core/src/main/create-app.ts index d0cccae5f8..57f764706f 100644 --- a/packages/core/src/main/create-app.ts +++ b/packages/core/src/main/create-app.ts @@ -3,20 +3,17 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -import type { DiContainer } from "@ogre-tools/injectable"; import { getInjectable } from "@ogre-tools/injectable"; import { runInAction } from "mobx"; +import type { CreateApplication } from "../common/create-app"; import nodeEnvInjectionToken from "../common/vars/node-env-injection-token"; +import { getDi } from "./getDi"; import { registerInjectables } from "./register-injectables"; import startMainApplicationInjectable from "./start-main-application/start-main-application.injectable"; -interface AppConfig { - di: DiContainer; - mode: string; -} - -export function createApp(conf: AppConfig) { - const { di, mode } = conf; +export const createApplication: CreateApplication = (config) => { + const { mode } = config; + const di = getDi(); runInAction(() => { di.register(getInjectable({ @@ -28,9 +25,8 @@ export function createApp(conf: AppConfig) { registerInjectables(di); }); - const startMainApplication = di.inject(startMainApplicationInjectable); - return { - start: () => startMainApplication(), + start: di.inject(startMainApplicationInjectable), + di, }; -} +}; diff --git a/packages/core/src/main/getDi.ts b/packages/core/src/main/getDi.ts index e325e66ed5..8d4b1ff57f 100644 --- a/packages/core/src/main/getDi.ts +++ b/packages/core/src/main/getDi.ts @@ -3,5 +3,15 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ import { createContainer } from "@ogre-tools/injectable"; +import { registerMobX } from "@ogre-tools/injectable-extension-for-mobx"; +import { setLegacyGlobalDiForExtensionApi } from "../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; -export const getDi = () => createContainer("main"); +export const getDi = () => { + const environment = "main"; + const di = createContainer(environment); + + registerMobX(di); + setLegacyGlobalDiForExtensionApi(di, environment); + + return di; +}; diff --git a/packages/core/src/main/getDiForUnitTesting.ts b/packages/core/src/main/getDiForUnitTesting.ts index 5e923433fa..1e4c3291b9 100644 --- a/packages/core/src/main/getDiForUnitTesting.ts +++ b/packages/core/src/main/getDiForUnitTesting.ts @@ -4,9 +4,8 @@ */ import { chunk } from "lodash/fp"; -import type { DiContainer, Injectable } from "@ogre-tools/injectable"; -import { createContainer, isInjectable, getInjectable } from "@ogre-tools/injectable"; -import { Environments, setLegacyGlobalDiForExtensionApi } from "../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; +import type { DiContainer } from "@ogre-tools/injectable"; +import { isInjectable } from "@ogre-tools/injectable"; import spawnInjectable from "./child-process/spawn.injectable"; import initializeExtensionsInjectable from "./start-main-application/runnables/initialize-extensions.injectable"; import setupIpcMainHandlersInjectable from "./electron-app/runnables/setup-ipc-main-handlers/setup-ipc-main-handlers.injectable"; @@ -25,46 +24,30 @@ import electronQuitAndInstallUpdateInjectable from "./electron-app/features/elec import electronUpdaterIsActiveInjectable from "./electron-app/features/electron-updater-is-active.injectable"; import setUpdateOnQuitInjectable from "./electron-app/features/set-update-on-quit.injectable"; import waitUntilBundledExtensionsAreLoadedInjectable from "./start-main-application/lens-window/application-window/wait-until-bundled-extensions-are-loaded.injectable"; -import { registerMobX } from "@ogre-tools/injectable-extension-for-mobx"; import electronInjectable from "./utils/resolve-system-proxy/electron.injectable"; import initializeClusterManagerInjectable from "./cluster/initialize-manager.injectable"; import type { GlobalOverride } from "../common/test-utils/get-global-override"; -import nodeEnvInjectionToken from "../common/vars/node-env-injection-token"; import { getOverrideFsWithFakes } from "../test-utils/override-fs-with-fakes"; -import { applicationInformationFakeInjectable } from "../common/vars/application-information-fake-injectable"; +import { getDi } from "./getDi"; export function getDiForUnitTesting(opts: { doGeneralOverrides?: boolean } = {}) { const { doGeneralOverrides = false, } = opts; - const di = createContainer("main"); - - di.register(getInjectable({ - id: "node-env", - instantiate: () => "production", - injectionToken: nodeEnvInjectionToken, - })); - - setLegacyGlobalDiForExtensionApi(di, Environments.main); + const di = getDi(); di.preventSideEffects(); - const injectables = ( - global.injectablePaths.main.paths + runInAction(() => { + const injectables = global.injectablePaths.main.paths .map(path => require(path)) .flatMap(Object.values) - .filter(isInjectable) - ) as Injectable[]; + .filter(isInjectable); - registerMobX(di); - - runInAction(() => { - di.register(applicationInformationFakeInjectable); - - chunk(100)(injectables).forEach(chunkInjectables => { - di.register(...chunkInjectables); - }); + for (const block of chunk(100)(injectables)) { + di.register(...block); + } }); if (doGeneralOverrides) { diff --git a/packages/core/src/main/library.ts b/packages/core/src/main/library.ts index a8b4507b69..ab1f56528e 100644 --- a/packages/core/src/main/library.ts +++ b/packages/core/src/main/library.ts @@ -8,7 +8,8 @@ export { afterApplicationIsLoadedInjectionToken } from "./start-main-application export { beforeApplicationIsLoadingInjectionToken } from "./start-main-application/runnable-tokens/before-application-is-loading-injection-token"; export { beforeElectronIsReadyInjectionToken } from "./start-main-application/runnable-tokens/before-electron-is-ready-injection-token"; export { onLoadOfApplicationInjectionToken } from "./start-main-application/runnable-tokens/on-load-of-application-injection-token"; -export { createApp } from "./create-app"; +export { createApplication } from "./create-app"; +export type { CreateApplication, Application, ApplicationConfig } from "../common/create-app"; export * as Mobx from "mobx"; export * as mainExtensionApi from "../extensions/main-api"; export * as commonExtensionApi from "../extensions/common-api"; diff --git a/packages/core/src/main/register-injectables.ts b/packages/core/src/main/register-injectables.ts index 5ddcb640c1..eb18df3d3f 100644 --- a/packages/core/src/main/register-injectables.ts +++ b/packages/core/src/main/register-injectables.ts @@ -4,16 +4,10 @@ */ import type { DiContainer } from "@ogre-tools/injectable"; import { autoRegister } from "@ogre-tools/injectable-extension-for-auto-registration"; -import { registerMobX } from "@ogre-tools/injectable-extension-for-mobx"; import { runInAction } from "mobx"; -import { Environments, setLegacyGlobalDiForExtensionApi } from "../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; export function registerInjectables(di: DiContainer) { - setLegacyGlobalDiForExtensionApi(di, Environments.main); - runInAction(() => { - registerMobX(di); - autoRegister({ di, targetModule: module, @@ -25,6 +19,4 @@ export function registerInjectables(di: DiContainer) { ], }); }); - - return di; } diff --git a/packages/core/src/renderer/create-app.ts b/packages/core/src/renderer/create-app.ts index 3a511af5ec..e2064a7ae2 100644 --- a/packages/core/src/renderer/create-app.ts +++ b/packages/core/src/renderer/create-app.ts @@ -5,19 +5,16 @@ import "./components/app.scss"; import { bootstrap } from "./bootstrap"; -import type { DiContainer } from "@ogre-tools/injectable"; import { getInjectable } from "@ogre-tools/injectable"; import nodeEnvInjectionToken from "../common/vars/node-env-injection-token"; import { runInAction } from "mobx"; import { registerInjectables } from "./register-injectables"; +import type { CreateApplication } from "../common/create-app"; +import { getDi } from "./getDi"; -interface AppConfig { - di: DiContainer; - mode: string; -} - -export function createApp(conf: AppConfig) { - const { di, mode } = conf; +export const createApplication: CreateApplication = (config) => { + const { mode } = config; + const di = getDi(); runInAction(() => { di.register(getInjectable({ @@ -25,10 +22,12 @@ export function createApp(conf: AppConfig) { instantiate: () => mode, injectionToken: nodeEnvInjectionToken, })); + registerInjectables(di); }); - + return { start: () => bootstrap(di), + di, }; -} +}; diff --git a/packages/core/src/renderer/getDi.tsx b/packages/core/src/renderer/getDi.tsx index 77d665760c..22e545e780 100644 --- a/packages/core/src/renderer/getDi.tsx +++ b/packages/core/src/renderer/getDi.tsx @@ -4,5 +4,17 @@ */ import { createContainer } from "@ogre-tools/injectable"; +import { registerMobX } from "@ogre-tools/injectable-extension-for-mobx"; +import { registerInjectableReact } from "@ogre-tools/injectable-react"; +import { setLegacyGlobalDiForExtensionApi } from "../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; -export const getDi = () => createContainer("renderer"); +export const getDi = () => { + const environment = "renderer"; + const di = createContainer(environment); + + registerMobX(di); + registerInjectableReact(di); + setLegacyGlobalDiForExtensionApi(di, environment); + + return di; +}; diff --git a/packages/core/src/renderer/getDiForUnitTesting.tsx b/packages/core/src/renderer/getDiForUnitTesting.tsx index 15ddef9804..e1f6186874 100644 --- a/packages/core/src/renderer/getDiForUnitTesting.tsx +++ b/packages/core/src/renderer/getDiForUnitTesting.tsx @@ -4,9 +4,7 @@ */ import { noop, chunk } from "lodash/fp"; -import type { Injectable } from "@ogre-tools/injectable"; -import { createContainer, isInjectable, getInjectable } from "@ogre-tools/injectable"; -import { Environments, setLegacyGlobalDiForExtensionApi } from "../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; +import { isInjectable } from "@ogre-tools/injectable"; import requestFromChannelInjectable from "./utils/channel/request-from-channel.injectable"; import { getOverrideFsWithFakes } from "../test-utils/override-fs-with-fakes"; import terminalSpawningPoolInjectable from "./components/dock/terminal/terminal-spawning-pool.injectable"; @@ -14,47 +12,29 @@ import hostedClusterIdInjectable from "./cluster-frame-context/hosted-cluster-id import { runInAction } from "mobx"; import requestAnimationFrameInjectable from "./components/animate/request-animation-frame.injectable"; import startTopbarStateSyncInjectable from "./components/layout/top-bar/start-state-sync.injectable"; -import { registerMobX } from "@ogre-tools/injectable-extension-for-mobx"; import watchHistoryStateInjectable from "./remote-helpers/watch-history-state.injectable"; import legacyOnChannelListenInjectable from "./ipc/legacy-channel-listen.injectable"; import type { GlobalOverride } from "../common/test-utils/get-global-override"; -import nodeEnvInjectionToken from "../common/vars/node-env-injection-token"; -import { applicationInformationFakeInjectable } from "../common/vars/application-information-fake-injectable"; -import { registerInjectableReact } from "@ogre-tools/injectable-react"; +import { getDi } from "./getDi"; export const getDiForUnitTesting = ( opts: { doGeneralOverrides?: boolean } = {}, ) => { const { doGeneralOverrides = false } = opts; - const di = createContainer("renderer"); - - di.register(getInjectable({ - id: "node-env", - instantiate: () => "production", - injectionToken: nodeEnvInjectionToken, - })); + const di = getDi(); di.preventSideEffects(); - setLegacyGlobalDiForExtensionApi(di, Environments.renderer); - - const injectables = ( - global.injectablePaths.renderer.paths + runInAction(() => { + const injectables = global.injectablePaths.renderer.paths .map(path => require(path)) .flatMap(Object.values) - .filter(isInjectable) - ) as Injectable[]; + .filter(isInjectable); - registerMobX(di); - registerInjectableReact(di); - - runInAction(() => { - di.register(applicationInformationFakeInjectable); - - chunk(100)(injectables).forEach((chunkInjectables) => { - di.register(...chunkInjectables); - }); + for (const block of chunk(100)(injectables)) { + di.register(...block); + } }); if (doGeneralOverrides) { diff --git a/packages/core/src/renderer/library.ts b/packages/core/src/renderer/library.ts index be72fc6c89..aa9c1fc907 100644 --- a/packages/core/src/renderer/library.ts +++ b/packages/core/src/renderer/library.ts @@ -14,4 +14,5 @@ export * as ReactRouter from "react-router"; export * as ReactRouterDom from "react-router-dom"; export * as rendererExtensionApi from "../extensions/renderer-api"; export * as commonExtensionApi from "../extensions/common-api"; -export { createApp } from "./create-app"; +export { createApplication } from "./create-app"; +export type { CreateApplication, Application, ApplicationConfig } from "../common/create-app"; diff --git a/packages/core/src/renderer/register-injectables.ts b/packages/core/src/renderer/register-injectables.ts index 41724d9413..0974a1a08b 100644 --- a/packages/core/src/renderer/register-injectables.ts +++ b/packages/core/src/renderer/register-injectables.ts @@ -5,19 +5,10 @@ import type { DiContainer } from "@ogre-tools/injectable"; import { autoRegister } from "@ogre-tools/injectable-extension-for-auto-registration"; -import { registerMobX } from "@ogre-tools/injectable-extension-for-mobx"; -import { registerInjectableReact } from "@ogre-tools/injectable-react"; import { runInAction } from "mobx"; -import { Environments, setLegacyGlobalDiForExtensionApi } from "../extensions/as-legacy-globals-for-extension-api/legacy-global-di-for-extension-api"; export function registerInjectables(di: DiContainer) { - setLegacyGlobalDiForExtensionApi(di, Environments.renderer); - - registerMobX(di); - registerInjectableReact(di); - runInAction(() => { - autoRegister({ di, targetModule: module, @@ -29,6 +20,4 @@ export function registerInjectables(di: DiContainer) { ], }); }); - - return di; } diff --git a/packages/open-lens/src/main/index.ts b/packages/open-lens/src/main/index.ts index aca3da2bb4..39500b2240 100644 --- a/packages/open-lens/src/main/index.ts +++ b/packages/open-lens/src/main/index.ts @@ -1,18 +1,15 @@ -import { createContainer } from "@ogre-tools/injectable"; import { autoRegister } from "@ogre-tools/injectable-extension-for-auto-registration"; import { runInAction } from "mobx"; -import { createApp, mainExtensionApi as Main, commonExtensionApi as Common } from "@k8slens/core/main"; +import { createApplication, mainExtensionApi as Main, commonExtensionApi as Common } from "@k8slens/core/main"; -const di = createContainer("main"); -const app = createApp({ - di, +const app = createApplication({ mode: process.env.NODE_ENV || "development" }); runInAction(() => { try { autoRegister({ - di, + di: app.di, targetModule: module, getRequireContexts: () => [ require.context("./", true, CONTEXT_MATCHER_FOR_NON_FEATURES), diff --git a/packages/open-lens/src/renderer/index.ts b/packages/open-lens/src/renderer/index.ts index 2774644e94..ed28f26d3a 100644 --- a/packages/open-lens/src/renderer/index.ts +++ b/packages/open-lens/src/renderer/index.ts @@ -1,18 +1,15 @@ import "@k8slens/core/styles"; -import { createContainer } from "@ogre-tools/injectable"; import { runInAction } from "mobx"; -import { createApp, rendererExtensionApi as Renderer, commonExtensionApi as Common } from "@k8slens/core/renderer"; +import { createApplication, rendererExtensionApi as Renderer, commonExtensionApi as Common } from "@k8slens/core/renderer"; import { autoRegister } from "@ogre-tools/injectable-extension-for-auto-registration"; -const di = createContainer("renderer"); -const app = createApp({ - di, +const app = createApplication({ mode: process.env.NODE_ENV || "development" }); runInAction(() => { autoRegister({ - di, + di: app.di, targetModule: module, getRequireContexts: () => [ require.context("./", true, CONTEXT_MATCHER_FOR_NON_FEATURES),