refactor: spfx package question (#9301)

* refactor: spfx question * test: ut1 * test: ut * refactor: spfx * test: ut
OfficeDev · Jul 17, 2023 · 42d5f74 · 42d5f74
1 parent 235582c
commit 42d5f74
Show file tree

Hide file tree

Showing 10 changed files with 147 additions and 272 deletions.
diff --git a/packages/api/review/teamsfx-api.api.md b/packages/api/review/teamsfx-api.api.md
@@ -259,6 +259,10 @@ export interface Inputs extends Record<string, any> {
     // (undocumented)
     existingResources?: string[];
     // (undocumented)
+    globalSpfxPackageVersion?: string;
+    // (undocumented)
+    globalYeomanPackageVersion?: string;
+    // (undocumented)
     ignoreConfigPersist?: boolean;
     // (undocumented)
     ignoreEnvInfo?: boolean;
@@ -267,6 +271,8 @@ export interface Inputs extends Record<string, any> {
     // (undocumented)
     isM365?: boolean;
     // (undocumented)
+    latestSpfxPackageVersion?: string;
+    // (undocumented)
     locale?: string;
     // (undocumented)
     openAIPluginManifest?: OpenAIPluginManifest;

diff --git a/packages/api/src/types.ts b/packages/api/src/types.ts
@@ -85,6 +85,9 @@ export interface Inputs extends Record<string, any> {
   inProductDoc?: boolean; // AB test for in product doc feature
   teamsAppFromTdp?: any;
   openAIPluginManifest?: OpenAIPluginManifest;
+  globalYeomanPackageVersion?: string;
+  globalSpfxPackageVersion?: string;
+  latestSpfxPackageVersion?: string;
 }
 
 export type InputsWithProjectPath = Inputs & { projectPath: string };

diff --git a/packages/fx-core/src/component/generator/spfx/depsChecker/generatorChecker.ts b/packages/fx-core/src/component/generator/spfx/depsChecker/generatorChecker.ts
@@ -22,7 +22,6 @@ import { DependencyValidateError, NpmInstallError } from "../error";
 import { cpUtils } from "../../../../common/deps-checker/util/cpUtils";
 import { Constants } from "../utils/constants";
 import { getExecCommand, Utils } from "../utils/utils";
-import { PackageSelectOptionsHelper } from "../../../../question/create";
 
 const name = Constants.GeneratorPackageName;
 const displayName = `${name}@${Constants.LatestVersion}`;
@@ -62,12 +61,10 @@ export class GeneratorChecker implements DependencyChecker {
     return ok(true);
   }
 
-  public async isLatestInstalled(): Promise<boolean> {
+  public async isLatestInstalled(loadedLatestVersion: string | undefined): Promise<boolean> {
     try {
       const generatorVersion = await this.queryVersion();
-      const latestGeneratorVersion =
-        PackageSelectOptionsHelper.getLatestSpGeneratorVersion() ??
-        (await this.findLatestVersion(5));
+      const latestGeneratorVersion = loadedLatestVersion ?? (await this.findLatestVersion(5));
       const hasSentinel = await fs.pathExists(this.getSentinelPath());
       return !!latestGeneratorVersion && generatorVersion === latestGeneratorVersion && hasSentinel;
     } catch (error) {

diff --git a/packages/fx-core/src/component/generator/spfx/spfxGenerator.ts b/packages/fx-core/src/component/generator/spfx/spfxGenerator.ts
@@ -44,9 +44,10 @@ import {
 } from "./error";
 import { Constants, ManifestTemplate } from "./utils/constants";
 import { ProgressHelper } from "./utils/progress-helper";
-import { PackageSelectOptionsHelper, SPFxVersionOptionIds } from "../../../question/create";
+import { SPFxVersionOptionIds } from "../../../question/create";
 import { TelemetryEvents, TelemetryProperty } from "./utils/telemetryEvents";
 import { Utils } from "./utils/utils";
+import semver from "semver";
 
 export class SPFxGenerator {
   @hooks([
@@ -232,7 +233,9 @@ export class SPFxGenerator {
 
       if (shouldInstallLocally) {
         const latestYoInstalled = await yoChecker.isLatestInstalled();
-        const latestGeneratorInstalled = await spGeneratorChecker.isLatestInstalled();
+        const latestGeneratorInstalled = await spGeneratorChecker.isLatestInstalled(
+          inputs.latestSpfxPackageVersion
+        );
 
         if (!latestYoInstalled || !latestGeneratorInstalled) {
           await progressHandler?.next(
@@ -254,7 +257,12 @@ export class SPFxGenerator {
           }
         }
       } else {
-        const isLowerVersion = PackageSelectOptionsHelper.isLowerThanRecommendedVersion();
+        const isLowerVersion =
+          !!inputs.globalSpfxPackageVersion &&
+          semver.lte(
+            inputs.globalSpfxPackageVersion,
+            Constants.RecommendedLowestSpfxVersion.substring(1)
+          );
         if (isLowerVersion) {
           context.telemetryReporter.sendTelemetryEvent(TelemetryEvents.UseNotRecommendedVersion);
         }

diff --git a/packages/fx-core/src/question/create.ts b/packages/fx-core/src/question/create.ts
@@ -4,7 +4,6 @@
 import {
   CLIPlatforms,
   FolderQuestion,
-  FuncQuestion,
   IQTreeNode,
   Inputs,
   MultiSelectQuestion,
@@ -21,7 +20,6 @@ import * as jsonschema from "jsonschema";
 import { cloneDeep } from "lodash";
 import * as os from "os";
 import * as path from "path";
-import semver from "semver";
 import { ConstantString } from "../common/constants";
 import { isCLIDotNetEnabled, isCopilotPluginEnabled } from "../common/featureFlags";
 import { getLocalizedString } from "../common/localizeUtils";
@@ -720,14 +718,56 @@ export function SPFxPackageSelectQuestion(): SingleSelectQuestion {
     staticOptions: [],
     placeholder: getLocalizedString("plugins.spfx.questions.packageSelect.placeholder"),
     dynamicOptions: async (inputs: Inputs): Promise<OptionItem[]> => {
-      await PackageSelectOptionsHelper.loadOptions();
-      return PackageSelectOptionsHelper.getOptions();
+      const versions = await Promise.all([
+        Utils.findGloballyInstalledVersion(undefined, Constants.GeneratorPackageName, 0, false),
+        Utils.findLatestVersion(undefined, Constants.GeneratorPackageName, 5),
+        Utils.findGloballyInstalledVersion(undefined, Constants.YeomanPackageName, 0, false),
+      ]);
+
+      inputs.globalSpfxPackageVersion = versions[0];
+      inputs.latestSpfxPackageVersion = versions[1];
+      inputs.globalYeomanPackageVersion = versions[2];
+
+      return [
+        {
+          id: SPFxVersionOptionIds.installLocally,
+
+          label:
+            versions[1] !== undefined
+              ? getLocalizedString(
+                  "plugins.spfx.questions.packageSelect.installLocally.withVersion.label",
+                  "v" + versions[1]
+                )
+              : getLocalizedString(
+                  "plugins.spfx.questions.packageSelect.installLocally.noVersion.label"
+                ),
+        },
+        {
+          id: SPFxVersionOptionIds.globalPackage,
+          label:
+            versions[0] !== undefined
+              ? getLocalizedString(
+                  "plugins.spfx.questions.packageSelect.useGlobalPackage.withVersion.label",
+                  "v" + versions[0]
+                )
+              : getLocalizedString(
+                  "plugins.spfx.questions.packageSelect.useGlobalPackage.noVersion.label"
+                ),
+          description: getLocalizedString(
+            "plugins.spfx.questions.packageSelect.useGlobalPackage.detail",
+            Constants.RecommendedLowestSpfxVersion
+          ),
+        },
+      ];
     },
     default: SPFxVersionOptionIds.installLocally,
     validation: {
-      validFunc: async (input: string): Promise<string | undefined> => {
+      validFunc: async (input: string, previousInputs?: Inputs): Promise<string | undefined> => {
         if (input === SPFxVersionOptionIds.globalPackage) {
-          const hasPackagesInstalled = PackageSelectOptionsHelper.checkGlobalPackages();
+          const hasPackagesInstalled =
+            !!previousInputs &&
+            !!previousInputs.globalSpfxPackageVersion &&
+            !!previousInputs.globalYeomanPackageVersion;
           if (!hasPackagesInstalled) {
             throw DevEnvironmentSetupError();
           }
@@ -802,86 +842,6 @@ export enum SPFxVersionOptionIds {
   globalPackage = "false",
 }
 
-export class PackageSelectOptionsHelper {
-  private static options: OptionItem[] = [];
-  private static globalPackageVersions: (string | undefined)[] = [undefined, undefined];
-  private static latestSpGeneratorVersion: string | undefined = undefined;
-
-  public static async loadOptions(): Promise<void> {
-    const versions = await Promise.all([
-      Utils.findGloballyInstalledVersion(undefined, Constants.GeneratorPackageName, 0, false),
-      Utils.findLatestVersion(undefined, Constants.GeneratorPackageName, 5),
-      Utils.findGloballyInstalledVersion(undefined, Constants.YeomanPackageName, 0, false),
-    ]);
-
-    PackageSelectOptionsHelper.globalPackageVersions[0] = versions[0];
-    PackageSelectOptionsHelper.globalPackageVersions[1] = versions[2];
-    PackageSelectOptionsHelper.latestSpGeneratorVersion = versions[1];
-
-    PackageSelectOptionsHelper.options = [
-      {
-        id: SPFxVersionOptionIds.installLocally,
-
-        label:
-          versions[1] !== undefined
-            ? getLocalizedString(
-                "plugins.spfx.questions.packageSelect.installLocally.withVersion.label",
-                "v" + versions[1]
-              )
-            : getLocalizedString(
-                "plugins.spfx.questions.packageSelect.installLocally.noVersion.label"
-              ),
-      },
-      {
-        id: SPFxVersionOptionIds.globalPackage,
-        label:
-          versions[0] !== undefined
-            ? getLocalizedString(
-                "plugins.spfx.questions.packageSelect.useGlobalPackage.withVersion.label",
-                "v" + versions[0]
-              )
-            : getLocalizedString(
-                "plugins.spfx.questions.packageSelect.useGlobalPackage.noVersion.label"
-              ),
-        description: getLocalizedString(
-          "plugins.spfx.questions.packageSelect.useGlobalPackage.detail",
-          Constants.RecommendedLowestSpfxVersion
-        ),
-      },
-    ];
-  }
-
-  public static getOptions(): OptionItem[] {
-    return PackageSelectOptionsHelper.options;
-  }
-
-  public static clear(): void {
-    PackageSelectOptionsHelper.options = [];
-    PackageSelectOptionsHelper.globalPackageVersions = [undefined, undefined];
-    PackageSelectOptionsHelper.latestSpGeneratorVersion = undefined;
-  }
-
-  public static checkGlobalPackages(): boolean {
-    return (
-      !!PackageSelectOptionsHelper.globalPackageVersions[0] &&
-      !!PackageSelectOptionsHelper.globalPackageVersions[1]
-    );
-  }
-
-  public static getLatestSpGeneratorVersion(): string | undefined {
-    return PackageSelectOptionsHelper.latestSpGeneratorVersion;
-  }
-
-  public static isLowerThanRecommendedVersion(): boolean | undefined {
-    const installedVersion = PackageSelectOptionsHelper.globalPackageVersions[0];
-    if (!installedVersion) {
-      return undefined;
-    }
-
-    const recommendedLowestVersion = Constants.RecommendedLowestSpfxVersion.substring(1); // remove "v"
-    return semver.lte(installedVersion, recommendedLowestVersion);
-  }
-}
 export function SPFxImportFolderQuestion(hasDefaultFunc = false): FolderQuestion {
   return {
     type: "folder",

diff --git a/packages/fx-core/tests/component/generator/spfxGenerator.test.ts b/packages/fx-core/tests/component/generator/spfxGenerator.test.ts
@@ -19,11 +19,7 @@ import { Utils } from "../../../src/component/generator/spfx/utils/utils";
 import { createContextV3 } from "../../../src/component/utils";
 import { envUtil } from "../../../src/component/utils/envUtil";
 import { setTools } from "../../../src/core/globalVars";
-import {
-  PackageSelectOptionsHelper,
-  QuestionNames,
-  SPFxVersionOptionIds,
-} from "../../../src/question";
+import { QuestionNames, SPFxVersionOptionIds } from "../../../src/question";
 import { MockTools } from "../../core/utils";
 
 describe("SPFxGenerator", function () {
@@ -384,9 +380,9 @@ describe("SPFxGenerator", function () {
       [QuestionNames.AppName]: "spfxTestApp",
       [QuestionNames.SPFxInstallPackage]: SPFxVersionOptionIds.globalPackage,
       [QuestionNames.SPFxSolution]: "new",
+      globalSpfxPackageVersion: "1.17.0",
     };
     sinon.stub(YoChecker.prototype, "isLatestInstalled").resolves(true);
-    sinon.stub(PackageSelectOptionsHelper, "isLowerThanRecommendedVersion").resolves(true);
     sinon.stub(GeneratorChecker.prototype, "isLatestInstalled").resolves(true);
     sinon.stub(cpUtils, "executeCommand").resolves("succeed");
 

diff --git a/packages/fx-core/tests/plugins/resource/spfx/depsChecker/generatorChecker.test.ts b/packages/fx-core/tests/plugins/resource/spfx/depsChecker/generatorChecker.test.ts
@@ -12,7 +12,6 @@ import { telemetryHelper } from "../../../../../src/component/generator/spfx/uti
 import { createContextV3 } from "../../../../../src/component/utils";
 import { setTools } from "../../../../../src/core/globalVars";
 import { MockTools } from "../../../../core/utils";
-import { PackageSelectOptionsHelper } from "../../../../../src/question/create";
 
 class StubLogger implements LogProvider {
   async log(logLevel: LogLevel, message: string): Promise<boolean> {
@@ -63,7 +62,6 @@ describe("generator checker", () => {
 
   afterEach(() => {
     restore();
-    PackageSelectOptionsHelper.clear();
   });
 
   describe("getDependencyInfo", async () => {
@@ -161,7 +159,7 @@ describe("generator checker", () => {
         return "latest";
       });
 
-      const result = await checker.isLatestInstalled();
+      const result = await checker.isLatestInstalled("latest");
       chai.expect(result).is.true;
     });
 
@@ -182,28 +180,7 @@ describe("generator checker", () => {
         return "latest";
       });
 
-      const result = await checker.isLatestInstalled();
-      chai.expect(result).is.false;
-    });
-
-    it("latest not installed", async () => {
-      const checker = new GeneratorChecker(new StubLogger());
-      stub(fs, "pathExists").callsFake(async () => {
-        console.log("stub pathExists");
-        return false;
-      });
-
-      stub(GeneratorChecker.prototype, <any>"queryVersion").callsFake(async () => {
-        console.log("stub queryversion");
-        return "lower version";
-      });
-
-      stub(GeneratorChecker.prototype, <any>"findLatestVersion").callsFake(async () => {
-        console.log("stub findLatestVersion");
-        return "latest";
-      });
-
-      const result = await checker.isLatestInstalled();
+      const result = await checker.isLatestInstalled(undefined);
       chai.expect(result).is.false;
     });
 
@@ -216,7 +193,7 @@ describe("generator checker", () => {
 
       stub(GeneratorChecker.prototype, <any>"queryVersion").throws("error");
 
-      const result = await checker.isLatestInstalled();
+      const result = await checker.isLatestInstalled(undefined);
       chai.expect(result).is.false;
     });
   });