Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning for custom build scripts in project's package.json #5240

1 change: 1 addition & 0 deletions CHANGELOG.md
@@ -1,2 +1,3 @@
- Add support for object list using certain Admin SDKs (#5208)
- Fixes source token expiration issue by acquiring new source token upon expiration.
- 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(
chalosalvador marked this conversation as resolved.
Show resolved Hide resolved
`\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`
);
});
});
});