Skip to content

Commit

Permalink
Warning for custom build scripts in project's package.json (#5240)
Browse files Browse the repository at this point in the history
Warn if the build command in the source directory's package.json contains anything other than the default build command. Direct users towards the Express.js / custom integration
  • Loading branch information
jamesdaniels committed Dec 9, 2022
1 parent 97ce550 commit d7f0186
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
@@ -1,2 +1,3 @@
- Add support for Firestore TTL (#5267)
- Fix bug where secrets were not loaded when emulating functions with `--inpsect-functions`. (#4605)
- Warn if a web framework's package.json contains anything other than the framework default build command.
4 changes: 4 additions & 0 deletions src/frameworks/angular/index.ts
Expand Up @@ -14,12 +14,14 @@ import {
} from "..";
import { promptOnce } from "../../prompt";
import { proxyRequestHandler } from "../../hosting/proxy";
import { warnIfCustomBuildScript } from "../utils";

export const name = "Angular";
export const support = SupportLevel.Experimental;
export const type = FrameworkType.Framework;

const CLI_COMMAND = join("node_modules", ".bin", process.platform === "win32" ? "ng.cmd" : "ng");
const DEFAULT_BUILD_SCRIPT = ["ng build"];

export async function discover(dir: string): Promise<Discovery | undefined> {
if (!(await pathExists(join(dir, "package.json")))) return;
Expand Down Expand Up @@ -57,6 +59,8 @@ export async function build(dir: string): Promise<BuildResult> {
if (!success) throw new Error(error);
};

await warnIfCustomBuildScript(dir, name, DEFAULT_BUILD_SCRIPT);

if (!browserTarget) throw new Error("No build target...");

if (prerenderTarget) {
Expand Down
5 changes: 5 additions & 0 deletions src/frameworks/next/index.ts
Expand Up @@ -22,6 +22,7 @@ import { IncomingMessage, ServerResponse } from "http";
import { logger } from "../../logger";
import { FirebaseError } from "../../error";
import { fileExistsSync } from "../../fsutils";
import { warnIfCustomBuildScript } from "../utils";

// Next.js's exposed interface is incomplete here
// TODO see if there's a better way to grab this
Expand All @@ -45,6 +46,8 @@ const CLI_COMMAND = join(
process.platform === "win32" ? "next.cmd" : "next"
);

const DEFAULT_BUILD_SCRIPT = ["next build"];

export const name = "Next.js";
export const support = SupportLevel.Experimental;
export const type = FrameworkType.MetaFramework;
Expand Down Expand Up @@ -73,6 +76,8 @@ export async function discover(dir: string) {
export async function build(dir: string): Promise<BuildResult> {
const { default: nextBuild } = relativeRequire(dir, "next/dist/build");

await warnIfCustomBuildScript(dir, name, DEFAULT_BUILD_SCRIPT);

const reactVersion = getReactVersion(dir);
if (reactVersion && gte(reactVersion, "18.0.0")) {
// This needs to be set for Next build to succeed with React 18
Expand Down
6 changes: 6 additions & 0 deletions src/frameworks/nuxt/index.ts
Expand Up @@ -3,11 +3,14 @@ import { readFile } from "fs/promises";
import { basename, join } from "path";
import { gte } from "semver";
import { BuildResult, findDependency, FrameworkType, relativeRequire, SupportLevel } from "..";
import { warnIfCustomBuildScript } from "../utils";

export const name = "Nuxt";
export const support = SupportLevel.Experimental;
export const type = FrameworkType.Toolchain;

const DEFAULT_BUILD_SCRIPT = ["nuxt build"];

export async function discover(dir: string) {
if (!(await pathExists(join(dir, "package.json")))) return;
const nuxtDependency = findDependency("nuxt", { cwd: dir, depth: 0, omitDev: false });
Expand All @@ -23,6 +26,9 @@ export async function discover(dir: string) {
export async function build(root: string): Promise<BuildResult> {
const { buildNuxt } = await relativeRequire(root, "@nuxt/kit");
const nuxtApp = await getNuxtApp(root);

await warnIfCustomBuildScript(root, name, DEFAULT_BUILD_SCRIPT);

await buildNuxt(nuxtApp);
return { wantsBackend: true };
}
Expand Down
22 changes: 22 additions & 0 deletions src/frameworks/utils.ts
@@ -0,0 +1,22 @@
import { readFile } from "fs/promises";
import { join } from "path";

/**
* Prints a warning if the build script in package.json
* contains anything other than allowedBuildScripts.
*/
export async function warnIfCustomBuildScript(
dir: string,
framework: string,
defaultBuildScripts: string[]
): Promise<void> {
const packageJsonBuffer = await readFile(join(dir, "package.json"));
const packageJson = JSON.parse(packageJsonBuffer.toString());
const buildScript = packageJson.scripts?.build;

if (buildScript && !defaultBuildScripts.includes(buildScript)) {
console.warn(
`\nWARNING: Your package.json contains a custom build that is being ignored. Only the ${framework} default build script (e.g, "${defaultBuildScripts[0]}") is respected. If you have a more advanced build process you should build a custom integration https://firebase.google.com/docs/hosting/express\n`
);
}
}
6 changes: 6 additions & 0 deletions src/frameworks/vite/index.ts
Expand Up @@ -5,6 +5,7 @@ import { join } from "path";
import { findDependency, FrameworkType, relativeRequire, SupportLevel } from "..";
import { proxyRequestHandler } from "../../hosting/proxy";
import { promptOnce } from "../../prompt";
import { warnIfCustomBuildScript } from "../utils";

export const name = "Vite";
export const support = SupportLevel.Experimental;
Expand All @@ -16,6 +17,8 @@ const CLI_COMMAND = join(
process.platform === "win32" ? "vite.cmd" : "vite"
);

export const DEFAULT_BUILD_SCRIPT = ["vite build", "tsc && vite build"];

export const initViteTemplate = (template: string) => async (setup: any) =>
await init(setup, template);

Expand Down Expand Up @@ -61,6 +64,9 @@ export async function discover(dir: string, plugin?: string, npmDependency?: str

export async function build(root: string) {
const { build } = relativeRequire(root, "vite");

await warnIfCustomBuildScript(root, name, DEFAULT_BUILD_SCRIPT);

await build({ root });
}

Expand Down
53 changes: 53 additions & 0 deletions src/test/frameworks/utils.spec.ts
@@ -0,0 +1,53 @@
import { expect } from "chai";
import * as sinon from "sinon";
import * as fs from "fs";

import { warnIfCustomBuildScript } from "../../frameworks/utils";

describe("Frameworks utils", () => {
describe("warnIfCustomBuildScript", () => {
const framework = "Next.js";
let sandbox: sinon.SinonSandbox;
let consoleLogSpy: sinon.SinonSpy;
const packageJson = {
scripts: {
build: "",
},
};

beforeEach(() => {
sandbox = sinon.createSandbox();
consoleLogSpy = sandbox.spy(console, "warn");
});

afterEach(() => {
sandbox.restore();
});

it("should not print warning when a default build script is found.", async () => {
const buildScript = "next build";
const defaultBuildScripts = ["next build"];
packageJson.scripts.build = buildScript;

sandbox.stub(fs.promises, "readFile").resolves(JSON.stringify(packageJson));

await warnIfCustomBuildScript("fakedir/", framework, defaultBuildScripts);

expect(consoleLogSpy.callCount).to.equal(0);
});

it("should print warning when a custom build script is found.", async () => {
const buildScript = "echo 'Custom build script' && next build";
const defaultBuildScripts = ["next build"];
packageJson.scripts.build = buildScript;

sandbox.stub(fs.promises, "readFile").resolves(JSON.stringify(packageJson));

await warnIfCustomBuildScript("fakedir/", framework, defaultBuildScripts);

expect(consoleLogSpy).to.be.calledOnceWith(
`\nWARNING: Your package.json contains a custom build that is being ignored. Only the ${framework} default build script (e.g, "${defaultBuildScripts[0]}") is respected. If you have a more advanced build process you should build a custom integration https://firebase.google.com/docs/hosting/express\n`
);
});
});
});

0 comments on commit d7f0186

Please sign in to comment.