From be779e07105ea8efdec4afbbd380a91a147add88 Mon Sep 17 00:00:00 2001 From: Sairam Sakhamuri Date: Thu, 15 Jun 2023 14:38:09 -0700 Subject: [PATCH 01/10] Added runtime command discovery --- .../compose/discover/runtime/node.ts | 276 +++++++++++++++ .../compose/discover/runtime/node.spec.ts | 322 ++++++++++++++++++ 2 files changed, 598 insertions(+) create mode 100644 src/frameworks/compose/discover/runtime/node.ts create mode 100644 src/test/frameworks/compose/discover/runtime/node.spec.ts diff --git a/src/frameworks/compose/discover/runtime/node.ts b/src/frameworks/compose/discover/runtime/node.ts new file mode 100644 index 00000000000..f59e0af9089 --- /dev/null +++ b/src/frameworks/compose/discover/runtime/node.ts @@ -0,0 +1,276 @@ +import { readOrNull } from "../filesystem"; +import { FileSystem, FrameworkSpec, Runtime } from "../types"; +import { RuntimeSpec } from "../types"; +import { frameworkMatcher } from "../frameworkMatcher"; +import { LifecycleCommands } from "../types"; +import { Command } from "../types"; +import { join } from "path"; +import { logger } from "../../../../logger"; +import { FirebaseError } from "../../../../error"; +import { VALID_ENGINES } from "../../../constants"; +import { conjoinOptions } from "../../../utils"; +export interface PackageJSON { + dependencies?: Record; + devDependencies?: Record; + scripts?: Record; + engines?: Record; +} + +const NODE_RUNTIME_ID = "nodejs"; +const PACKAGE_JSON = "package.json"; +const PACKAGE_LOCK_JSON = "package-lock.json"; +const YARN = "yarn"; +const YARN_LOCK = "yarn.lock"; +const NPM = "npm"; + +export class NodejsRuntime implements Runtime { + private readonly runtimeRequiredFiles: string[] = [PACKAGE_JSON]; + private readonly contentCache: Record = {}; + + // Checks if the codebase is using Node as runtime. + async match(fs: FileSystem): Promise { + const areAllFilesPresent = await Promise.all( + this.runtimeRequiredFiles.map((file) => fs.exists(file)) + ); + + return Promise.resolve(areAllFilesPresent.every((present) => present)); + } + + getRuntimeName(): string { + return NODE_RUNTIME_ID; + } + + getNodeImage(engine: Record | undefined): string { + // If no version is mentioned explicitly, assuming application is compatible with latest version. + if (!engine) { + const latest = VALID_ENGINES.node[VALID_ENGINES.node.length - 1]; + return `node:${latest}-slim`; + } + const versionNumber = parseInt(engine.node, 10); + const validEngines = VALID_ENGINES.node.filter((it) => it !== versionNumber); + + if (!validEngines.length) { + throw new FirebaseError( + `This integration expects Node version ${conjoinOptions( + VALID_ENGINES.node, + "or" + )}. You're running version ${versionNumber}, which is not compatible.` + ); + } + + return `node:${versionNumber}-slim`; + } + + async getPackageManager(fs: FileSystem): Promise { + if (await fs.exists(YARN_LOCK)) { + return YARN; + } + + return NPM; + } + + async getDependenciesForNPM( + fs: FileSystem, + packageJSON: PackageJSON + ): Promise> { + const directDependencies = { ...packageJSON.dependencies, ...packageJSON.devDependencies }; + let transitiveDependencies = {}; + + const packageLockJSONRaw = await readOrNull(fs, PACKAGE_LOCK_JSON); + if (!packageLockJSONRaw) { + return directDependencies; + } + const packageLockJSON = JSON.parse(packageLockJSONRaw); + const directDependencyNames = Object.keys(directDependencies).map((x) => + join("node_modules", x) + ); + + directDependencyNames.forEach((directDepName) => { + const transitiveDeps = packageLockJSON.packages[directDepName].dependencies; + transitiveDependencies = { ...transitiveDependencies, ...transitiveDeps }; + }); + + return { ...directDependencies, ...transitiveDependencies }; + } + + async getDependenciesForYARN( + fs: FileSystem, + packageJSON: PackageJSON + ): Promise> { + const directDependencies = { ...packageJSON.dependencies, ...packageJSON.devDependencies }; + const yarnLockJSONRaw = await readOrNull(fs, YARN_LOCK); + if (!yarnLockJSONRaw) { + return directDependencies; + } + + const allDependencies: any = {}; + const lines = yarnLockJSONRaw.split("\n"); + + for (let line of lines) { + line = line.trim(); + if (line.startsWith("#") || line === "") { + continue; + } + const patternMatch = /^"(.+?)@(.+?)":/.exec(line); + if (patternMatch) { + const dependencyName = patternMatch[1]; + const version = patternMatch[2]; + allDependencies[dependencyName] = version; + } + } + + return allDependencies; + } + + async getDependencies( + fs: FileSystem, + packageJSON: PackageJSON, + packageManager: string + ): Promise> { + try { + let dependencies = {}; + if (packageManager === NPM) { + dependencies = await this.getDependenciesForNPM(fs, packageJSON); + } else if (packageManager === YARN) { + dependencies = await this.getDependenciesForYARN(fs, packageJSON); + } + + return dependencies; + } catch (error: any) { + logger.error("Error while reading dependencies for the project: ", error); + throw error; + } + } + + packageManagerInstallCommand(packageManager: string): string | undefined { + const packages: string[] = []; + if (packageManager === YARN) { + packages.push(YARN); + } + if (!packages.length) { + return undefined; + } + + return `npm install --global ${packages.join(" ")}`; + } + + installCommand(packageManager: string): string | undefined { + if (packageManager === NPM) { + return "npm ci"; + } else if (packageManager === YARN) { + return "yarn install"; + } + + return undefined; + } + + detectedCommands( + packageManager: string, + scripts: Record | undefined, + matchedFramework: FrameworkSpec | null + ): LifecycleCommands { + return { + build: this.getBuildCommand(packageManager, scripts, matchedFramework), + dev: this.getDevCommand(packageManager, scripts, matchedFramework), + run: this.getRunCommand(packageManager, scripts, matchedFramework), + }; + } + + executeScript(packageManager: string, scriptName: string): string { + return `${packageManager} run ${scriptName}`; + } + + executeFrameworkCommand(packageManager: string, command: Command): Command { + if (packageManager === NPM || packageManager === YARN) { + command.cmd = "npx " + command.cmd; + } + + return command; + } + + getBuildCommand( + packageManager: string, + scripts: Record | undefined, + matchedFramework: FrameworkSpec | null + ): Command { + let buildCommand: Command = { cmd: "" }; + if (scripts?.build) { + buildCommand.cmd = this.executeScript(packageManager, "build"); + } else if (matchedFramework && matchedFramework.commands?.build) { + buildCommand = matchedFramework.commands.build; + buildCommand = this.executeFrameworkCommand(packageManager, buildCommand); + } + + return buildCommand; + } + + getDevCommand( + packageManager: string, + scripts: Record | undefined, + matchedFramework: FrameworkSpec | null + ): Command { + let devCommand: Command = { cmd: "", env: { NODE_ENV: "dev" } }; + if (scripts?.dev) { + devCommand.cmd = this.executeScript(packageManager, "dev"); + } else if (matchedFramework && matchedFramework.commands?.dev) { + devCommand = matchedFramework.commands.dev; + devCommand = this.executeFrameworkCommand(packageManager, devCommand); + } + + return devCommand; + } + + getRunCommand( + packageManager: string, + scripts: Record | undefined, + matchedFramework: FrameworkSpec | null + ): Command { + let runCommand: Command = { cmd: "", env: { NODE_ENV: "production" } }; + if (scripts?.start) { + runCommand.cmd = this.executeScript(packageManager, "start"); + } else if (matchedFramework && matchedFramework.commands?.run) { + runCommand = matchedFramework.commands.run; + runCommand = this.executeFrameworkCommand(packageManager, runCommand); + } + + return runCommand; + } + + async analyseCodebase( + fs: FileSystem, + allFrameworkSpecs: FrameworkSpec[] + ): Promise { + try { + const packageJSONRaw = await readOrNull(fs, PACKAGE_JSON); + if (!packageJSONRaw) { + return null; + } + const packageJSON = JSON.parse(packageJSONRaw) as PackageJSON; + const packageManager = await this.getPackageManager(fs); + const nodeImage = this.getNodeImage(packageJSON.engines); + const dependencies = await this.getDependencies(fs, packageJSON, packageManager); + const matchedFramework = await frameworkMatcher( + NODE_RUNTIME_ID, + fs, + allFrameworkSpecs, + dependencies + ); + + const runtimeSpec: RuntimeSpec = { + id: NODE_RUNTIME_ID, + baseImage: nodeImage, + packageManagerInstallCommand: this.packageManagerInstallCommand(packageManager), + installCommand: this.installCommand(packageManager), + detectedCommands: this.detectedCommands( + packageManager, + packageJSON.scripts, + matchedFramework + ), + }; + + return runtimeSpec; + } catch (error: any) { + throw new FirebaseError(`Failed to indentify commands for codebase: ${error}`); + } + } +} diff --git a/src/test/frameworks/compose/discover/runtime/node.spec.ts b/src/test/frameworks/compose/discover/runtime/node.spec.ts new file mode 100644 index 00000000000..d106979a0ba --- /dev/null +++ b/src/test/frameworks/compose/discover/runtime/node.spec.ts @@ -0,0 +1,322 @@ +import { MockFileSystem } from "../mockFileSystem"; +import { expect } from "chai"; +import { + NodejsRuntime, + PackageJSON, +} from "../../../../../frameworks/compose/discover/runtime/node"; +import { FrameworkSpec } from "../../../../../frameworks/compose/discover/types"; + +describe("NodejsRuntime", () => { + let nodeJSRuntime: NodejsRuntime; + + before(() => { + nodeJSRuntime = new NodejsRuntime(); + }); + + describe("getNodeImage", () => { + it("should return a valid node Image", () => { + const version: Record = { + node: "18", + }; + const actualImage = nodeJSRuntime.getNodeImage(version); + const expectedImage = "node:18-slim"; + + expect(actualImage).to.deep.equal(expectedImage); + }); + + it("should return node Image", () => { + const version: Record = { + node: "16", + }; + const actualImage = nodeJSRuntime.getNodeImage(version); + const expectedImage = "node:16-slim"; + + expect(actualImage).to.deep.equal(expectedImage); + }); + }); + + describe("getPackageManager", () => { + it("should return yarn package manager", async () => { + const fileSystem = new MockFileSystem({ + "yarn.lock": "It is test file", + }); + const actual = await nodeJSRuntime.getPackageManager(fileSystem); + const expected = "yarn"; + + expect(actual).to.equal(expected); + }); + }); + + describe("getDependenciesNPM", () => { + it("should return direct and transitive dependencies", async () => { + const fileSystem = new MockFileSystem({ + "package.json": JSON.stringify({ + dependencies: { + express: "^4.18.2", + }, + devDependencies: { + nodemon: "^2.0.12", + mocha: "^9.1.1", + }, + }), + "package-lock.json": JSON.stringify({ + packages: { + "node_modules/express": { + dependencies: { + accepts: "~1.3.8", + "array-flatten": "1.1.1", + }, + }, + "node_modules/nodemon": { + dependencies: { + chokidar: "^3.5.2", + debug: "^3.2.7", + }, + }, + "node_modules/mocha": { + dependencies: { + "escape-string-regexp": "4.0.0", + "find-up": "5.0.0", + }, + }, + }, + }), + }); + const packageJSON: PackageJSON = { + dependencies: { + express: "^4.18.2", + }, + devDependencies: { + nodemon: "^2.0.12", + mocha: "^9.1.1", + }, + }; + const actual = await nodeJSRuntime.getDependencies(fileSystem, packageJSON, "npm"); + const expected = { + express: "^4.18.2", + nodemon: "^2.0.12", + mocha: "^9.1.1", + accepts: "~1.3.8", + "array-flatten": "1.1.1", + chokidar: "^3.5.2", + debug: "^3.2.7", + "escape-string-regexp": "4.0.0", + "find-up": "5.0.0", + }; + + expect(actual).to.deep.equal(expected); + }); + }); + + describe("getDependenciesYARN", () => { + it("should return all dependencies present in yarn file", async () => { + const fileSystem = new MockFileSystem({ + "package.json": JSON.stringify({ + dependencies: { + express: "^4.18.2", + }, + devDependencies: { + "@algolia/autocomplete-core": "npm:1.7.1", + mocha: "^9.1.1", + }, + }), + "yarn.lock": ` + # Testing + __metadata: + version: 7 + cacheKey: 9 + "express@^4.18.2": + version "^4.18.2" + resolved "https://registry.yarnpkg.com/axios/-/axios-0.21.1.tgz" + integrity sha512-abc123... + "@algolia/autocomplete-core@npm:1.7.1": + version "npm:1.7.1" + resolved "https://registry.yarnpkg.com/ms/-/ms-2.1.3.tgz" + integrity sha512-xyz456... + "mocha@^9.1.1": + version "9.1.1" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.0.2.tgz" + integrity sha512-pqr789... + `, + }); + const packageJSON: PackageJSON = { + dependencies: { + express: "^4.18.2", + }, + devDependencies: { + "@algolia/autocomplete-core": "npm:1.7.1", + mocha: "^9.1.1", + }, + }; + const actual = await nodeJSRuntime.getDependencies(fileSystem, packageJSON, "yarn"); + const expected = { + express: "^4.18.2", + "@algolia/autocomplete-core": "npm:1.7.1", + mocha: "^9.1.1", + }; + + expect(actual).to.deep.equal(expected); + }); + }); + + describe("detectedCommands", () => { + it("scripts are run using yarn and framework commands prepend npx", () => { + const matchedFramework: FrameworkSpec = { + id: "next", + runtime: "nodejs", + requiredDependencies: [], + commands: { + dev: { + cmd: "next dev", + env: { NODE_ENV: "dev" }, + }, + }, + }; + const scripts = { + build: "next build", + start: "next start", + }; + + const actual = nodeJSRuntime.detectedCommands("yarn", scripts, matchedFramework); + const expected = { + build: { + cmd: "yarn run build", + }, + dev: { + cmd: "npx next dev", + env: { NODE_ENV: "dev" }, + }, + run: { + cmd: "yarn run start", + env: { NODE_ENV: "production" }, + }, + }; + + expect(actual).to.deep.equal(expected); + }); + + it("scripts have higher preference over framework commands", () => { + const matchedFramework: FrameworkSpec = { + id: "next", + runtime: "nodejs", + requiredDependencies: [], + commands: { + build: { + cmd: "next build testing", + }, + run: { + cmd: "next start testing", + env: { NODE_ENV: "production" }, + }, + dev: { + cmd: "next dev", + env: { NODE_ENV: "dev" }, + }, + }, + }; + const scripts = { + build: "next build", + start: "next start", + }; + + const actual = nodeJSRuntime.detectedCommands("yarn", scripts, matchedFramework); + const expected = { + build: { + cmd: "yarn run build", + }, + dev: { + cmd: "npx next dev", + env: { NODE_ENV: "dev" }, + }, + run: { + cmd: "yarn run start", + env: { NODE_ENV: "production" }, + }, + }; + + expect(actual).to.deep.equal(expected); + }); + }); + + describe("analyseCodebase", () => { + it("should return runtime specs", async () => { + const fileSystem = new MockFileSystem({ + "next.config.js": "For testing", + "next.config.ts": "For testing", + "package.json": JSON.stringify({ + scripts: { + build: "next build", + start: "next start", + }, + dependencies: { + next: "13.4.5", + react: "18.2.0", + }, + engines: { + node: "18", + }, + }), + "package-lock.json": JSON.stringify({ + packages: { + "node_modules/next": { + dependencies: { + accepts: "~1.3.8", + "array-flatten": "1.1.1", + }, + }, + "node_modules/react": { + dependencies: { + chokidar: "^3.5.2", + debug: "^3.2.7", + }, + }, + }, + }), + }); + + const allFrameworks: FrameworkSpec[] = [ + { + id: "express", + runtime: "nodejs", + requiredDependencies: [{ name: "express" }], + }, + { + id: "next", + runtime: "nodejs", + requiredDependencies: [{ name: "next" }], + requiredFiles: [["next.config.js"], "next.config.ts"], + embedsFrameworks: ["react"], + commands: { + dev: { + cmd: "next dev", + env: { NODE_ENV: "dev" }, + }, + }, + }, + ]; + + const actual = await nodeJSRuntime.analyseCodebase(fileSystem, allFrameworks); + const expected = { + id: "nodejs", + baseImage: "node:18-slim", + packageManagerInstallCommand: undefined, + installCommand: "npm ci", + detectedCommands: { + build: { + cmd: "npm run build", + }, + dev: { + cmd: "npx next dev", + env: { NODE_ENV: "dev" }, + }, + run: { + cmd: "npm run start", + env: { NODE_ENV: "production" }, + }, + }, + }; + + expect(actual).to.deep.equal(expected); + }); + }); +}); From c1db2738a636844ab1b24eefd5d30107bd204275 Mon Sep 17 00:00:00 2001 From: Sairam Sakhamuri Date: Mon, 19 Jun 2023 14:00:33 -0700 Subject: [PATCH 02/10] Resolved comments --- .../compose/discover/runtime/node.ts | 130 ++++-------------- .../compose/discover/runtime/node.spec.ts | 112 +-------------- 2 files changed, 32 insertions(+), 210 deletions(-) diff --git a/src/frameworks/compose/discover/runtime/node.ts b/src/frameworks/compose/discover/runtime/node.ts index f59e0af9089..34d5ddbd5e7 100644 --- a/src/frameworks/compose/discover/runtime/node.ts +++ b/src/frameworks/compose/discover/runtime/node.ts @@ -4,24 +4,20 @@ import { RuntimeSpec } from "../types"; import { frameworkMatcher } from "../frameworkMatcher"; import { LifecycleCommands } from "../types"; import { Command } from "../types"; -import { join } from "path"; -import { logger } from "../../../../logger"; import { FirebaseError } from "../../../../error"; -import { VALID_ENGINES } from "../../../constants"; -import { conjoinOptions } from "../../../utils"; + export interface PackageJSON { dependencies?: Record; devDependencies?: Record; scripts?: Record; engines?: Record; } +type PackageManager = "npm" | "yarn"; +const supportedNodeVersion = 18; const NODE_RUNTIME_ID = "nodejs"; const PACKAGE_JSON = "package.json"; -const PACKAGE_LOCK_JSON = "package-lock.json"; -const YARN = "yarn"; const YARN_LOCK = "yarn.lock"; -const NPM = "npm"; export class NodejsRuntime implements Runtime { private readonly runtimeRequiredFiles: string[] = [PACKAGE_JSON]; @@ -43,109 +39,35 @@ export class NodejsRuntime implements Runtime { getNodeImage(engine: Record | undefined): string { // If no version is mentioned explicitly, assuming application is compatible with latest version. if (!engine) { - const latest = VALID_ENGINES.node[VALID_ENGINES.node.length - 1]; - return `node:${latest}-slim`; + return `node:${supportedNodeVersion}-slim`; } const versionNumber = parseInt(engine.node, 10); - const validEngines = VALID_ENGINES.node.filter((it) => it !== versionNumber); - if (!validEngines.length) { + if (versionNumber !== supportedNodeVersion) { throw new FirebaseError( - `This integration expects Node version ${conjoinOptions( - VALID_ENGINES.node, - "or" - )}. You're running version ${versionNumber}, which is not compatible.` + `This integration expects Node version ${supportedNodeVersion}. You're running version ${versionNumber}, which is not compatible.` ); } return `node:${versionNumber}-slim`; } - async getPackageManager(fs: FileSystem): Promise { + async getPackageManager(fs: FileSystem): Promise { if (await fs.exists(YARN_LOCK)) { - return YARN; + return "yarn"; } - return NPM; + return "npm"; } - async getDependenciesForNPM( - fs: FileSystem, - packageJSON: PackageJSON - ): Promise> { - const directDependencies = { ...packageJSON.dependencies, ...packageJSON.devDependencies }; - let transitiveDependencies = {}; - - const packageLockJSONRaw = await readOrNull(fs, PACKAGE_LOCK_JSON); - if (!packageLockJSONRaw) { - return directDependencies; - } - const packageLockJSON = JSON.parse(packageLockJSONRaw); - const directDependencyNames = Object.keys(directDependencies).map((x) => - join("node_modules", x) - ); - - directDependencyNames.forEach((directDepName) => { - const transitiveDeps = packageLockJSON.packages[directDepName].dependencies; - transitiveDependencies = { ...transitiveDependencies, ...transitiveDeps }; - }); - - return { ...directDependencies, ...transitiveDependencies }; - } - - async getDependenciesForYARN( - fs: FileSystem, - packageJSON: PackageJSON - ): Promise> { - const directDependencies = { ...packageJSON.dependencies, ...packageJSON.devDependencies }; - const yarnLockJSONRaw = await readOrNull(fs, YARN_LOCK); - if (!yarnLockJSONRaw) { - return directDependencies; - } - - const allDependencies: any = {}; - const lines = yarnLockJSONRaw.split("\n"); - - for (let line of lines) { - line = line.trim(); - if (line.startsWith("#") || line === "") { - continue; - } - const patternMatch = /^"(.+?)@(.+?)":/.exec(line); - if (patternMatch) { - const dependencyName = patternMatch[1]; - const version = patternMatch[2]; - allDependencies[dependencyName] = version; - } - } - - return allDependencies; - } - - async getDependencies( - fs: FileSystem, - packageJSON: PackageJSON, - packageManager: string - ): Promise> { - try { - let dependencies = {}; - if (packageManager === NPM) { - dependencies = await this.getDependenciesForNPM(fs, packageJSON); - } else if (packageManager === YARN) { - dependencies = await this.getDependenciesForYARN(fs, packageJSON); - } - - return dependencies; - } catch (error: any) { - logger.error("Error while reading dependencies for the project: ", error); - throw error; - } + getDependencies(packageJSON: PackageJSON): Record { + return { ...packageJSON.dependencies, ...packageJSON.devDependencies }; } - packageManagerInstallCommand(packageManager: string): string | undefined { + packageManagerInstallCommand(packageManager: PackageManager): string | undefined { const packages: string[] = []; - if (packageManager === YARN) { - packages.push(YARN); + if (packageManager === "yarn") { + packages.push("yarn"); } if (!packages.length) { return undefined; @@ -154,10 +76,10 @@ export class NodejsRuntime implements Runtime { return `npm install --global ${packages.join(" ")}`; } - installCommand(packageManager: string): string | undefined { - if (packageManager === NPM) { - return "npm ci"; - } else if (packageManager === YARN) { + installCommand(packageManager: PackageManager): string | undefined { + if (packageManager === "npm") { + return "npm install"; + } else if (packageManager === "yarn") { return "yarn install"; } @@ -165,7 +87,7 @@ export class NodejsRuntime implements Runtime { } detectedCommands( - packageManager: string, + packageManager: PackageManager, scripts: Record | undefined, matchedFramework: FrameworkSpec | null ): LifecycleCommands { @@ -180,8 +102,8 @@ export class NodejsRuntime implements Runtime { return `${packageManager} run ${scriptName}`; } - executeFrameworkCommand(packageManager: string, command: Command): Command { - if (packageManager === NPM || packageManager === YARN) { + executeFrameworkCommand(packageManager: PackageManager, command: Command): Command { + if (packageManager === "npm" || packageManager === "yarn") { command.cmd = "npx " + command.cmd; } @@ -189,7 +111,7 @@ export class NodejsRuntime implements Runtime { } getBuildCommand( - packageManager: string, + packageManager: PackageManager, scripts: Record | undefined, matchedFramework: FrameworkSpec | null ): Command { @@ -205,7 +127,7 @@ export class NodejsRuntime implements Runtime { } getDevCommand( - packageManager: string, + packageManager: PackageManager, scripts: Record | undefined, matchedFramework: FrameworkSpec | null ): Command { @@ -221,7 +143,7 @@ export class NodejsRuntime implements Runtime { } getRunCommand( - packageManager: string, + packageManager: PackageManager, scripts: Record | undefined, matchedFramework: FrameworkSpec | null ): Command { @@ -248,7 +170,7 @@ export class NodejsRuntime implements Runtime { const packageJSON = JSON.parse(packageJSONRaw) as PackageJSON; const packageManager = await this.getPackageManager(fs); const nodeImage = this.getNodeImage(packageJSON.engines); - const dependencies = await this.getDependencies(fs, packageJSON, packageManager); + const dependencies = this.getDependencies(packageJSON); const matchedFramework = await frameworkMatcher( NODE_RUNTIME_ID, fs, @@ -270,7 +192,7 @@ export class NodejsRuntime implements Runtime { return runtimeSpec; } catch (error: any) { - throw new FirebaseError(`Failed to indentify commands for codebase: ${error}`); + throw new FirebaseError(`Failed to parse engine: ${error}`); } } } diff --git a/src/test/frameworks/compose/discover/runtime/node.spec.ts b/src/test/frameworks/compose/discover/runtime/node.spec.ts index d106979a0ba..185fb25a2b3 100644 --- a/src/test/frameworks/compose/discover/runtime/node.spec.ts +++ b/src/test/frameworks/compose/discover/runtime/node.spec.ts @@ -23,16 +23,6 @@ describe("NodejsRuntime", () => { expect(actualImage).to.deep.equal(expectedImage); }); - - it("should return node Image", () => { - const version: Record = { - node: "16", - }; - const actualImage = nodeJSRuntime.getNodeImage(version); - const expectedImage = "node:16-slim"; - - expect(actualImage).to.deep.equal(expectedImage); - }); }); describe("getPackageManager", () => { @@ -47,41 +37,8 @@ describe("NodejsRuntime", () => { }); }); - describe("getDependenciesNPM", () => { - it("should return direct and transitive dependencies", async () => { - const fileSystem = new MockFileSystem({ - "package.json": JSON.stringify({ - dependencies: { - express: "^4.18.2", - }, - devDependencies: { - nodemon: "^2.0.12", - mocha: "^9.1.1", - }, - }), - "package-lock.json": JSON.stringify({ - packages: { - "node_modules/express": { - dependencies: { - accepts: "~1.3.8", - "array-flatten": "1.1.1", - }, - }, - "node_modules/nodemon": { - dependencies: { - chokidar: "^3.5.2", - debug: "^3.2.7", - }, - }, - "node_modules/mocha": { - dependencies: { - "escape-string-regexp": "4.0.0", - "find-up": "5.0.0", - }, - }, - }, - }), - }); + describe("getDependencies", () => { + it("should return direct and transitive dependencies", () => { const packageJSON: PackageJSON = { dependencies: { express: "^4.18.2", @@ -91,68 +48,11 @@ describe("NodejsRuntime", () => { mocha: "^9.1.1", }, }; - const actual = await nodeJSRuntime.getDependencies(fileSystem, packageJSON, "npm"); + const actual = nodeJSRuntime.getDependencies(packageJSON); const expected = { express: "^4.18.2", nodemon: "^2.0.12", mocha: "^9.1.1", - accepts: "~1.3.8", - "array-flatten": "1.1.1", - chokidar: "^3.5.2", - debug: "^3.2.7", - "escape-string-regexp": "4.0.0", - "find-up": "5.0.0", - }; - - expect(actual).to.deep.equal(expected); - }); - }); - - describe("getDependenciesYARN", () => { - it("should return all dependencies present in yarn file", async () => { - const fileSystem = new MockFileSystem({ - "package.json": JSON.stringify({ - dependencies: { - express: "^4.18.2", - }, - devDependencies: { - "@algolia/autocomplete-core": "npm:1.7.1", - mocha: "^9.1.1", - }, - }), - "yarn.lock": ` - # Testing - __metadata: - version: 7 - cacheKey: 9 - "express@^4.18.2": - version "^4.18.2" - resolved "https://registry.yarnpkg.com/axios/-/axios-0.21.1.tgz" - integrity sha512-abc123... - "@algolia/autocomplete-core@npm:1.7.1": - version "npm:1.7.1" - resolved "https://registry.yarnpkg.com/ms/-/ms-2.1.3.tgz" - integrity sha512-xyz456... - "mocha@^9.1.1": - version "9.1.1" - resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.0.2.tgz" - integrity sha512-pqr789... - `, - }); - const packageJSON: PackageJSON = { - dependencies: { - express: "^4.18.2", - }, - devDependencies: { - "@algolia/autocomplete-core": "npm:1.7.1", - mocha: "^9.1.1", - }, - }; - const actual = await nodeJSRuntime.getDependencies(fileSystem, packageJSON, "yarn"); - const expected = { - express: "^4.18.2", - "@algolia/autocomplete-core": "npm:1.7.1", - mocha: "^9.1.1", }; expect(actual).to.deep.equal(expected); @@ -160,7 +60,7 @@ describe("NodejsRuntime", () => { }); describe("detectedCommands", () => { - it("scripts are run using yarn and framework commands prepend npx", () => { + it("should prepend npx to framework commands", () => { const matchedFramework: FrameworkSpec = { id: "next", runtime: "nodejs", @@ -195,7 +95,7 @@ describe("NodejsRuntime", () => { expect(actual).to.deep.equal(expected); }); - it("scripts have higher preference over framework commands", () => { + it("should prefer scripts over framework commands", () => { const matchedFramework: FrameworkSpec = { id: "next", runtime: "nodejs", @@ -300,7 +200,7 @@ describe("NodejsRuntime", () => { id: "nodejs", baseImage: "node:18-slim", packageManagerInstallCommand: undefined, - installCommand: "npm ci", + installCommand: "npm install", detectedCommands: { build: { cmd: "npm run build", From 497479be71e58b3eacfc78478fe211e7f8864e82 Mon Sep 17 00:00:00 2001 From: Sairam Sakhamuri Date: Mon, 19 Jun 2023 18:50:55 -0700 Subject: [PATCH 03/10] Added error case to analyse codebase method --- .../compose/discover/runtime/node.ts | 38 +++++++---- .../compose/discover/runtime/node.spec.ts | 65 ++++++++++++++----- 2 files changed, 75 insertions(+), 28 deletions(-) diff --git a/src/frameworks/compose/discover/runtime/node.ts b/src/frameworks/compose/discover/runtime/node.ts index 34d5ddbd5e7..a7d13229e13 100644 --- a/src/frameworks/compose/discover/runtime/node.ts +++ b/src/frameworks/compose/discover/runtime/node.ts @@ -5,6 +5,7 @@ import { frameworkMatcher } from "../frameworkMatcher"; import { LifecycleCommands } from "../types"; import { Command } from "../types"; import { FirebaseError } from "../../../../error"; +import { logger } from "../../../../../src/logger"; export interface PackageJSON { dependencies?: Record; @@ -18,6 +19,7 @@ const supportedNodeVersion = 18; const NODE_RUNTIME_ID = "nodejs"; const PACKAGE_JSON = "package.json"; const YARN_LOCK = "yarn.lock"; +const PACKAGE_LOCK_JSON = "package-lock.json"; export class NodejsRuntime implements Runtime { private readonly runtimeRequiredFiles: string[] = [PACKAGE_JSON]; @@ -53,11 +55,16 @@ export class NodejsRuntime implements Runtime { } async getPackageManager(fs: FileSystem): Promise { - if (await fs.exists(YARN_LOCK)) { - return "yarn"; - } + try { + if (await fs.exists(YARN_LOCK)) { + return "yarn"; + } - return "npm"; + return "npm"; + } catch (error: any) { + logger.error("Failed to check files to identify package manager"); + throw error; + } } getDependencies(packageJSON: PackageJSON): Record { @@ -76,14 +83,21 @@ export class NodejsRuntime implements Runtime { return `npm install --global ${packages.join(" ")}`; } - installCommand(packageManager: PackageManager): string | undefined { - if (packageManager === "npm") { - return "npm install"; - } else if (packageManager === "yarn") { - return "yarn install"; - } + async installCommand(fs: FileSystem, packageManager: PackageManager): Promise { + try { + let installCmd = "npm install"; + + if (await fs.exists(PACKAGE_LOCK_JSON)) { + installCmd = "npm ci"; + } else if (packageManager === "yarn") { + installCmd = "yarn install"; + } - return undefined; + return installCmd; + } catch (error: any) { + logger.error("Failed to check files to identify install command"); + throw error; + } } detectedCommands( @@ -182,7 +196,7 @@ export class NodejsRuntime implements Runtime { id: NODE_RUNTIME_ID, baseImage: nodeImage, packageManagerInstallCommand: this.packageManagerInstallCommand(packageManager), - installCommand: this.installCommand(packageManager), + installCommand: await this.installCommand(fs, packageManager), detectedCommands: this.detectedCommands( packageManager, packageJSON.scripts, diff --git a/src/test/frameworks/compose/discover/runtime/node.spec.ts b/src/test/frameworks/compose/discover/runtime/node.spec.ts index 185fb25a2b3..c1bcd613c6d 100644 --- a/src/test/frameworks/compose/discover/runtime/node.spec.ts +++ b/src/test/frameworks/compose/discover/runtime/node.spec.ts @@ -5,6 +5,7 @@ import { PackageJSON, } from "../../../../../frameworks/compose/discover/runtime/node"; import { FrameworkSpec } from "../../../../../frameworks/compose/discover/types"; +import { FirebaseError } from "../../../../../error"; describe("NodejsRuntime", () => { let nodeJSRuntime: NodejsRuntime; @@ -156,22 +157,6 @@ describe("NodejsRuntime", () => { node: "18", }, }), - "package-lock.json": JSON.stringify({ - packages: { - "node_modules/next": { - dependencies: { - accepts: "~1.3.8", - "array-flatten": "1.1.1", - }, - }, - "node_modules/react": { - dependencies: { - chokidar: "^3.5.2", - debug: "^3.2.7", - }, - }, - }, - }), }); const allFrameworks: FrameworkSpec[] = [ @@ -218,5 +203,53 @@ describe("NodejsRuntime", () => { expect(actual).to.deep.equal(expected); }); + + it("should return error", async () => { + const fileSystem = new MockFileSystem({ + "next.config.js": "For testing purpose.", + "next.config.ts": "For testing purpose.", + "package.json": JSON.stringify({ + scripts: { + build: "next build", + start: "next start", + }, + dependencies: { + express: "2.0.8", + next: "13.4.5", + react: "18.2.0", + }, + engines: { + node: "18", + }, + }), + }); + + const allFrameworks: FrameworkSpec[] = [ + { + id: "express", + runtime: "nodejs", + requiredDependencies: [{ name: "express" }], + }, + { + id: "next", + runtime: "nodejs", + requiredDependencies: [{ name: "next" }], + requiredFiles: [["next.config.js"], "next.config.ts"], + embedsFrameworks: ["react"], + commands: { + dev: { + cmd: "next dev", + env: { NODE_ENV: "dev" }, + }, + }, + }, + ]; + + // Failed with multiple framework matches + await expect(nodeJSRuntime.analyseCodebase(fileSystem, allFrameworks)).to.be.rejectedWith( + FirebaseError, + "Failed to parse engine" + ); + }); }); }); From 8652c14c7f29ac5a1d85a549c69b8e47d7bd6073 Mon Sep 17 00:00:00 2001 From: Sairam Sakhamuri Date: Tue, 20 Jun 2023 08:16:32 -0700 Subject: [PATCH 04/10] Updated install command --- src/frameworks/compose/discover/runtime/node.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/frameworks/compose/discover/runtime/node.ts b/src/frameworks/compose/discover/runtime/node.ts index a7d13229e13..b2a2b49f80e 100644 --- a/src/frameworks/compose/discover/runtime/node.ts +++ b/src/frameworks/compose/discover/runtime/node.ts @@ -86,11 +86,15 @@ export class NodejsRuntime implements Runtime { async installCommand(fs: FileSystem, packageManager: PackageManager): Promise { try { let installCmd = "npm install"; - if (await fs.exists(PACKAGE_LOCK_JSON)) { installCmd = "npm ci"; - } else if (packageManager === "yarn") { + } + + if (packageManager === "yarn") { installCmd = "yarn install"; + if (await fs.exists(YARN_LOCK)) { + installCmd = "yarn install --frozen-lockfile"; + } } return installCmd; From 9e936731f6df2deb8d913d67bffa131f515c2594 Mon Sep 17 00:00:00 2001 From: Sairam Sakhamuri Date: Wed, 21 Jun 2023 13:16:09 -0700 Subject: [PATCH 05/10] Reorganized tests and removed unwated promise.resolve stmt --- .../compose/discover/runtime/node.ts | 2 +- .../compose/discover/runtime/node.spec.ts | 64 +++++++------------ 2 files changed, 23 insertions(+), 43 deletions(-) diff --git a/src/frameworks/compose/discover/runtime/node.ts b/src/frameworks/compose/discover/runtime/node.ts index b2a2b49f80e..4e1234e8fa4 100644 --- a/src/frameworks/compose/discover/runtime/node.ts +++ b/src/frameworks/compose/discover/runtime/node.ts @@ -31,7 +31,7 @@ export class NodejsRuntime implements Runtime { this.runtimeRequiredFiles.map((file) => fs.exists(file)) ); - return Promise.resolve(areAllFilesPresent.every((present) => present)); + return areAllFilesPresent.every((present) => present); } getRuntimeName(): string { diff --git a/src/test/frameworks/compose/discover/runtime/node.spec.ts b/src/test/frameworks/compose/discover/runtime/node.spec.ts index c1bcd613c6d..395dd36e571 100644 --- a/src/test/frameworks/compose/discover/runtime/node.spec.ts +++ b/src/test/frameworks/compose/discover/runtime/node.spec.ts @@ -9,9 +9,30 @@ import { FirebaseError } from "../../../../../error"; describe("NodejsRuntime", () => { let nodeJSRuntime: NodejsRuntime; + let allFrameworks: FrameworkSpec[]; before(() => { nodeJSRuntime = new NodejsRuntime(); + allFrameworks = [ + { + id: "express", + runtime: "nodejs", + requiredDependencies: [{ name: "express" }], + }, + { + id: "next", + runtime: "nodejs", + requiredDependencies: [{ name: "next" }], + requiredFiles: [["next.config.js"], "next.config.ts"], + embedsFrameworks: ["react"], + commands: { + dev: { + cmd: "next dev", + env: { NODE_ENV: "dev" }, + }, + }, + }, + ]; }); describe("getNodeImage", () => { @@ -159,27 +180,6 @@ describe("NodejsRuntime", () => { }), }); - const allFrameworks: FrameworkSpec[] = [ - { - id: "express", - runtime: "nodejs", - requiredDependencies: [{ name: "express" }], - }, - { - id: "next", - runtime: "nodejs", - requiredDependencies: [{ name: "next" }], - requiredFiles: [["next.config.js"], "next.config.ts"], - embedsFrameworks: ["react"], - commands: { - dev: { - cmd: "next dev", - env: { NODE_ENV: "dev" }, - }, - }, - }, - ]; - const actual = await nodeJSRuntime.analyseCodebase(fileSystem, allFrameworks); const expected = { id: "nodejs", @@ -214,6 +214,7 @@ describe("NodejsRuntime", () => { start: "next start", }, dependencies: { + // Having both express and next as dependencies. express: "2.0.8", next: "13.4.5", react: "18.2.0", @@ -224,27 +225,6 @@ describe("NodejsRuntime", () => { }), }); - const allFrameworks: FrameworkSpec[] = [ - { - id: "express", - runtime: "nodejs", - requiredDependencies: [{ name: "express" }], - }, - { - id: "next", - runtime: "nodejs", - requiredDependencies: [{ name: "next" }], - requiredFiles: [["next.config.js"], "next.config.ts"], - embedsFrameworks: ["react"], - commands: { - dev: { - cmd: "next dev", - env: { NODE_ENV: "dev" }, - }, - }, - }, - ]; - // Failed with multiple framework matches await expect(nodeJSRuntime.analyseCodebase(fileSystem, allFrameworks)).to.be.rejectedWith( FirebaseError, From d28caaea0ebed6c8bc81c597cd570b8199dfbd0b Mon Sep 17 00:00:00 2001 From: Sairam Sakhamuri Date: Thu, 22 Jun 2023 14:34:35 -0700 Subject: [PATCH 06/10] Added review changes on install command and node version string array --- .../compose/discover/runtime/node.ts | 42 ++++++++----------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/src/frameworks/compose/discover/runtime/node.ts b/src/frameworks/compose/discover/runtime/node.ts index 4e1234e8fa4..f614e5f6f05 100644 --- a/src/frameworks/compose/discover/runtime/node.ts +++ b/src/frameworks/compose/discover/runtime/node.ts @@ -6,6 +6,7 @@ import { LifecycleCommands } from "../types"; import { Command } from "../types"; import { FirebaseError } from "../../../../error"; import { logger } from "../../../../../src/logger"; +import { conjoinOptions } from "../../../utils"; export interface PackageJSON { dependencies?: Record; @@ -15,11 +16,10 @@ export interface PackageJSON { } type PackageManager = "npm" | "yarn"; -const supportedNodeVersion = 18; +const supportedNodeVersions: string[] = ["18"]; const NODE_RUNTIME_ID = "nodejs"; const PACKAGE_JSON = "package.json"; const YARN_LOCK = "yarn.lock"; -const PACKAGE_LOCK_JSON = "package-lock.json"; export class NodejsRuntime implements Runtime { private readonly runtimeRequiredFiles: string[] = [PACKAGE_JSON]; @@ -40,14 +40,17 @@ export class NodejsRuntime implements Runtime { getNodeImage(engine: Record | undefined): string { // If no version is mentioned explicitly, assuming application is compatible with latest version. - if (!engine) { - return `node:${supportedNodeVersion}-slim`; + if (!engine || !engine.node) { + return `node:${supportedNodeVersions[supportedNodeVersions.length - 1]}-slim`; } - const versionNumber = parseInt(engine.node, 10); + const versionNumber = engine.node; - if (versionNumber !== supportedNodeVersion) { + if (!supportedNodeVersions.includes(versionNumber)) { throw new FirebaseError( - `This integration expects Node version ${supportedNodeVersion}. You're running version ${versionNumber}, which is not compatible.` + `This integration expects Node version ${conjoinOptions( + supportedNodeVersions, + "or" + )}. You're running version ${versionNumber}, which is not compatible.` ); } @@ -83,25 +86,14 @@ export class NodejsRuntime implements Runtime { return `npm install --global ${packages.join(" ")}`; } - async installCommand(fs: FileSystem, packageManager: PackageManager): Promise { - try { - let installCmd = "npm install"; - if (await fs.exists(PACKAGE_LOCK_JSON)) { - installCmd = "npm ci"; - } - - if (packageManager === "yarn") { - installCmd = "yarn install"; - if (await fs.exists(YARN_LOCK)) { - installCmd = "yarn install --frozen-lockfile"; - } - } + installCommand(fs: FileSystem, packageManager: PackageManager): string { + let installCmd = "npm install"; - return installCmd; - } catch (error: any) { - logger.error("Failed to check files to identify install command"); - throw error; + if (packageManager === "yarn") { + installCmd = "yarn install"; } + + return installCmd; } detectedCommands( @@ -200,7 +192,7 @@ export class NodejsRuntime implements Runtime { id: NODE_RUNTIME_ID, baseImage: nodeImage, packageManagerInstallCommand: this.packageManagerInstallCommand(packageManager), - installCommand: await this.installCommand(fs, packageManager), + installCommand: this.installCommand(fs, packageManager), detectedCommands: this.detectedCommands( packageManager, packageJSON.scripts, From 0e025c35374321a343720e6fb44e368f95554553 Mon Sep 17 00:00:00 2001 From: Sairam Sakhamuri Date: Mon, 26 Jun 2023 12:59:36 -0700 Subject: [PATCH 07/10] Changes to node.ts to include additional condions on run script --- .../compose/discover/runtime/node.ts | 36 ++++++++++--------- src/frameworks/compose/discover/types.ts | 13 ++++++- .../compose/discover/runtime/node.spec.ts | 14 +++++--- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/frameworks/compose/discover/runtime/node.ts b/src/frameworks/compose/discover/runtime/node.ts index f614e5f6f05..5463dfd47aa 100644 --- a/src/frameworks/compose/discover/runtime/node.ts +++ b/src/frameworks/compose/discover/runtime/node.ts @@ -96,15 +96,16 @@ export class NodejsRuntime implements Runtime { return installCmd; } - detectedCommands( + async detectedCommands( packageManager: PackageManager, scripts: Record | undefined, - matchedFramework: FrameworkSpec | null - ): LifecycleCommands { + matchedFramework: FrameworkSpec | null, + fs: FileSystem + ): Promise { return { build: this.getBuildCommand(packageManager, scripts, matchedFramework), dev: this.getDevCommand(packageManager, scripts, matchedFramework), - run: this.getRunCommand(packageManager, scripts, matchedFramework), + run: await this.getRunCommand(packageManager, scripts, matchedFramework, fs), }; } @@ -152,32 +153,34 @@ export class NodejsRuntime implements Runtime { return devCommand; } - getRunCommand( + async getRunCommand( packageManager: PackageManager, scripts: Record | undefined, - matchedFramework: FrameworkSpec | null - ): Command { + matchedFramework: FrameworkSpec | null, + fs: FileSystem + ): Promise { let runCommand: Command = { cmd: "", env: { NODE_ENV: "production" } }; if (scripts?.start) { runCommand.cmd = this.executeScript(packageManager, "start"); } else if (matchedFramework && matchedFramework.commands?.run) { runCommand = matchedFramework.commands.run; runCommand = this.executeFrameworkCommand(packageManager, runCommand); + } else if (scripts?.main) { + runCommand.cmd = `node ${scripts.main}`; + } else if (await fs.exists("index.js")) { + runCommand.cmd = `node index.js`; } return runCommand; } - async analyseCodebase( - fs: FileSystem, - allFrameworkSpecs: FrameworkSpec[] - ): Promise { + async analyseCodebase(fs: FileSystem, allFrameworkSpecs: FrameworkSpec[]): Promise { try { const packageJSONRaw = await readOrNull(fs, PACKAGE_JSON); - if (!packageJSONRaw) { - return null; + let packageJSON: PackageJSON = {}; + if (packageJSONRaw) { + packageJSON = JSON.parse(packageJSONRaw) as PackageJSON; } - const packageJSON = JSON.parse(packageJSONRaw) as PackageJSON; const packageManager = await this.getPackageManager(fs); const nodeImage = this.getNodeImage(packageJSON.engines); const dependencies = this.getDependencies(packageJSON); @@ -193,10 +196,11 @@ export class NodejsRuntime implements Runtime { baseImage: nodeImage, packageManagerInstallCommand: this.packageManagerInstallCommand(packageManager), installCommand: this.installCommand(fs, packageManager), - detectedCommands: this.detectedCommands( + detectedCommands: await this.detectedCommands( packageManager, packageJSON.scripts, - matchedFramework + matchedFramework, + fs ), }; diff --git a/src/frameworks/compose/discover/types.ts b/src/frameworks/compose/discover/types.ts index 677e054ec27..d90592ad301 100644 --- a/src/frameworks/compose/discover/types.ts +++ b/src/frameworks/compose/discover/types.ts @@ -1,3 +1,5 @@ +import { AppBundle } from "../interfaces"; + export interface FileSystem { exists(file: string): Promise; read(file: string): Promise; @@ -6,7 +8,7 @@ export interface FileSystem { export interface Runtime { match(fs: FileSystem): Promise; getRuntimeName(): string; - analyseCodebase(fs: FileSystem, allFrameworkSpecs: FrameworkSpec[]): Promise; + analyseCodebase(fs: FileSystem, allFrameworkSpecs: FrameworkSpec[]): Promise; } export interface Command { @@ -77,4 +79,13 @@ export interface RuntimeSpec { // The runtime has detected a command that should always be run irrespective of // the framework (e.g. the "build" script always wins in Node) detectedCommands?: LifecycleCommands; + + environmentVariables?: Record; + + frameworkHooks?: FrameworkHooks; +} + +export interface FrameworkHooks { + afterInstall?: (b: AppBundle) => AppBundle; + afterBuild?: (b: AppBundle) => AppBundle; } diff --git a/src/test/frameworks/compose/discover/runtime/node.spec.ts b/src/test/frameworks/compose/discover/runtime/node.spec.ts index 395dd36e571..b2bbaee767c 100644 --- a/src/test/frameworks/compose/discover/runtime/node.spec.ts +++ b/src/test/frameworks/compose/discover/runtime/node.spec.ts @@ -82,7 +82,10 @@ describe("NodejsRuntime", () => { }); describe("detectedCommands", () => { - it("should prepend npx to framework commands", () => { + it("should prepend npx to framework commands", async () => { + const fs = new MockFileSystem({ + "package.json": "Test file", + }); const matchedFramework: FrameworkSpec = { id: "next", runtime: "nodejs", @@ -99,7 +102,7 @@ describe("NodejsRuntime", () => { start: "next start", }; - const actual = nodeJSRuntime.detectedCommands("yarn", scripts, matchedFramework); + const actual = await nodeJSRuntime.detectedCommands("yarn", scripts, matchedFramework, fs); const expected = { build: { cmd: "yarn run build", @@ -117,7 +120,10 @@ describe("NodejsRuntime", () => { expect(actual).to.deep.equal(expected); }); - it("should prefer scripts over framework commands", () => { + it("should prefer scripts over framework commands", async () => { + const fs = new MockFileSystem({ + "package.json": "Test file", + }); const matchedFramework: FrameworkSpec = { id: "next", runtime: "nodejs", @@ -141,7 +147,7 @@ describe("NodejsRuntime", () => { start: "next start", }; - const actual = nodeJSRuntime.detectedCommands("yarn", scripts, matchedFramework); + const actual = await nodeJSRuntime.detectedCommands("yarn", scripts, matchedFramework, fs); const expected = { build: { cmd: "yarn run build", From 7ba91b9abd878003ffc4068a7dba67f079660d1a Mon Sep 17 00:00:00 2001 From: Sairam Sakhamuri Date: Mon, 26 Jun 2023 13:15:32 -0700 Subject: [PATCH 08/10] Added code comments to types --- src/frameworks/compose/discover/types.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/frameworks/compose/discover/types.ts b/src/frameworks/compose/discover/types.ts index d90592ad301..a89fb00c312 100644 --- a/src/frameworks/compose/discover/types.ts +++ b/src/frameworks/compose/discover/types.ts @@ -82,10 +82,16 @@ export interface RuntimeSpec { environmentVariables?: Record; + // Framework authors can execute framework-specific code using hooks at different stages of Frameworks API build process. frameworkHooks?: FrameworkHooks; } export interface FrameworkHooks { + // Programmatic hook with access to filesystem and nodejs API to inspect the workspace. + // Primarily intended to gather hints relevant to the build. afterInstall?: (b: AppBundle) => AppBundle; + + // Programmatic hook with access to filesystem and nodejs API to inspect the build artifacts. + // Primarily intended to informs what assets should be deployed. afterBuild?: (b: AppBundle) => AppBundle; } From e03ba8cdb071597e707afecc3157ba66f3baf677 Mon Sep 17 00:00:00 2001 From: Sairam Sakhamuri Date: Mon, 26 Jun 2023 15:58:55 -0700 Subject: [PATCH 09/10] Added undefied to return if no cmd --- src/frameworks/compose/discover/runtime/node.ts | 4 ++-- src/frameworks/compose/discover/types.ts | 17 ----------------- 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/src/frameworks/compose/discover/runtime/node.ts b/src/frameworks/compose/discover/runtime/node.ts index 5463dfd47aa..df9c94ebc87 100644 --- a/src/frameworks/compose/discover/runtime/node.ts +++ b/src/frameworks/compose/discover/runtime/node.ts @@ -158,7 +158,7 @@ export class NodejsRuntime implements Runtime { scripts: Record | undefined, matchedFramework: FrameworkSpec | null, fs: FileSystem - ): Promise { + ): Promise { let runCommand: Command = { cmd: "", env: { NODE_ENV: "production" } }; if (scripts?.start) { runCommand.cmd = this.executeScript(packageManager, "start"); @@ -171,7 +171,7 @@ export class NodejsRuntime implements Runtime { runCommand.cmd = `node index.js`; } - return runCommand; + return runCommand.cmd === "" ? undefined : runCommand; } async analyseCodebase(fs: FileSystem, allFrameworkSpecs: FrameworkSpec[]): Promise { diff --git a/src/frameworks/compose/discover/types.ts b/src/frameworks/compose/discover/types.ts index a89fb00c312..b919e552c4b 100644 --- a/src/frameworks/compose/discover/types.ts +++ b/src/frameworks/compose/discover/types.ts @@ -1,5 +1,3 @@ -import { AppBundle } from "../interfaces"; - export interface FileSystem { exists(file: string): Promise; read(file: string): Promise; @@ -79,19 +77,4 @@ export interface RuntimeSpec { // The runtime has detected a command that should always be run irrespective of // the framework (e.g. the "build" script always wins in Node) detectedCommands?: LifecycleCommands; - - environmentVariables?: Record; - - // Framework authors can execute framework-specific code using hooks at different stages of Frameworks API build process. - frameworkHooks?: FrameworkHooks; -} - -export interface FrameworkHooks { - // Programmatic hook with access to filesystem and nodejs API to inspect the workspace. - // Primarily intended to gather hints relevant to the build. - afterInstall?: (b: AppBundle) => AppBundle; - - // Programmatic hook with access to filesystem and nodejs API to inspect the build artifacts. - // Primarily intended to informs what assets should be deployed. - afterBuild?: (b: AppBundle) => AppBundle; } From 2edc9c98b2dbd74748cd3692f6d9ce09a1c803fc Mon Sep 17 00:00:00 2001 From: Sairam Sakhamuri Date: Mon, 26 Jun 2023 15:59:07 -0700 Subject: [PATCH 10/10] Added undefied to return if no cmd --- src/frameworks/compose/discover/runtime/node.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/frameworks/compose/discover/runtime/node.ts b/src/frameworks/compose/discover/runtime/node.ts index df9c94ebc87..976657deac9 100644 --- a/src/frameworks/compose/discover/runtime/node.ts +++ b/src/frameworks/compose/discover/runtime/node.ts @@ -125,7 +125,7 @@ export class NodejsRuntime implements Runtime { packageManager: PackageManager, scripts: Record | undefined, matchedFramework: FrameworkSpec | null - ): Command { + ): Command | undefined { let buildCommand: Command = { cmd: "" }; if (scripts?.build) { buildCommand.cmd = this.executeScript(packageManager, "build"); @@ -134,14 +134,14 @@ export class NodejsRuntime implements Runtime { buildCommand = this.executeFrameworkCommand(packageManager, buildCommand); } - return buildCommand; + return buildCommand.cmd === "" ? undefined : buildCommand; } getDevCommand( packageManager: PackageManager, scripts: Record | undefined, matchedFramework: FrameworkSpec | null - ): Command { + ): Command | undefined { let devCommand: Command = { cmd: "", env: { NODE_ENV: "dev" } }; if (scripts?.dev) { devCommand.cmd = this.executeScript(packageManager, "dev"); @@ -150,7 +150,7 @@ export class NodejsRuntime implements Runtime { devCommand = this.executeFrameworkCommand(packageManager, devCommand); } - return devCommand; + return devCommand.cmd === "" ? undefined : devCommand; } async getRunCommand(