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

Add supported range warnings #6562

Merged
merged 6 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
- Fixed an issue preventing Angular apps using ng-deploy from being emulated or deployed. (#6584)
- Warn if a Web Framework is outside a well known version range on deploy/emulate. (#6562)
- Use Web Framework's well known version range in `firebase init hosting`. (#6562)
8 changes: 6 additions & 2 deletions src/frameworks/angular/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
} from "../utils";
import {
getAllTargets,
getAngularVersion,
getBrowserConfig,
getBuildConfig,
getContext,
Expand All @@ -35,18 +36,21 @@

const DEFAULT_BUILD_SCRIPT = ["ng build"];

export const supportedRange = "14 - 17";

export async function discover(dir: string): Promise<Discovery | undefined> {

Check warning on line 41 in src/frameworks/angular/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment
if (!(await pathExists(join(dir, "package.json")))) return;
if (!(await pathExists(join(dir, "angular.json")))) return;
return { mayWantBackend: true, publicDirectory: join(dir, "src", "assets") };
const version = getAngularVersion(dir);
return { mayWantBackend: true, publicDirectory: join(dir, "src", "assets"), version };
}

export function init(setup: any, config: any) {

Check warning on line 48 in src/frameworks/angular/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function

Check warning on line 48 in src/frameworks/angular/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment

Check warning on line 48 in src/frameworks/angular/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type

Check warning on line 48 in src/frameworks/angular/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unexpected any. Specify a different type
execSync(
`npx --yes -p @angular/cli@latest ng new ${setup.projectId} --directory ${setup.hosting.source} --skip-git`,
`npx --yes -p @angular/cli@"${supportedRange}" ng new ${setup.projectId} --directory ${setup.hosting.source} --skip-git`,

Check warning on line 50 in src/frameworks/angular/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Invalid type "any" of template literal expression

Check warning on line 50 in src/frameworks/angular/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .projectId on an `any` value

Check warning on line 50 in src/frameworks/angular/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Invalid type "any" of template literal expression

Check warning on line 50 in src/frameworks/angular/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe member access .hosting on an `any` value
{
stdio: "inherit",
cwd: config.projectDir,

Check warning on line 53 in src/frameworks/angular/index.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value
}
);
return Promise.resolve();
Expand Down
17 changes: 16 additions & 1 deletion src/frameworks/angular/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ import type { ProjectDefinition } from "@angular-devkit/core/src/workspace";
import type { WorkspaceNodeModulesArchitectHost } from "@angular-devkit/architect/node";

import { AngularI18nConfig } from "./interfaces";
import { relativeRequire, validateLocales } from "../utils";
import { findDependency, relativeRequire, validateLocales } from "../utils";
import { FirebaseError } from "../../error";
import { join, posix, sep } from "path";
import { BUILD_TARGET_PURPOSE } from "../interfaces";
import { AssertionError } from "assert";
import { assertIsString } from "../../utils";
import { coerce } from "semver";

async function localesForTarget(
dir: string,
Expand Down Expand Up @@ -539,3 +540,17 @@ export async function getBuildConfig(sourceDir: string, configuration: string) {
ssr,
};
}

/**
* Get Angular version in the following format: `major.minor.patch`, ignoring
* canary versions as it causes issues with semver comparisons.
*/
export function getAngularVersion(cwd: string): string | undefined {
const dependency = findDependency("@angular/core", { cwd, depth: 0, omitDev: false });
if (!dependency) return undefined;

const angularVersionSemver = coerce(dependency.version);
if (!angularVersionSemver) return dependency.version;

return angularVersionSemver.toString();
}
5 changes: 4 additions & 1 deletion src/frameworks/astro/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@ import { getAstroVersion, getBootstrapScript, getConfig } from "./utils";
export const name = "Astro";
export const support = SupportLevel.Experimental;
export const type = FrameworkType.MetaFramework;
export const supportedRange = "2 - 3";

export async function discover(dir: string): Promise<Discovery | undefined> {
if (!existsSync(join(dir, "package.json"))) return;
if (!getAstroVersion(dir)) return;
const version = getAstroVersion(dir);
if (!version) return;
const { output, publicDir: publicDirectory } = await getConfig(dir);
return {
mayWantBackend: output !== "static",
publicDirectory,
version,
};
}

Expand Down
4 changes: 2 additions & 2 deletions src/frameworks/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ export const SupportLevelWarnings = {
[SupportLevel.Experimental]: (framework: string) => `Thank you for trying our ${clc.italic(
"experimental"
)} support for ${framework} on Firebase Hosting.
${clc.yellow(`While this integration is maintained by Googlers it is not a supported Firebase product.
${clc.red(`While this integration is maintained by Googlers it is not a supported Firebase product.
Issues filed on GitHub will be addressed on a best-effort basis by maintainers and other community members.`)}`,
[SupportLevel.Preview]: (framework: string) => `Thank you for trying our ${clc.italic(
"early preview"
)} of ${framework} support on Firebase Hosting.
${clc.yellow(
${clc.red(
"During the preview, support is best-effort and breaking changes can be expected. Proceed with caution."
)}`,
};
Expand Down
2 changes: 1 addition & 1 deletion src/frameworks/docs/nextjs.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ Using the {{firebase_cli}}, you can deploy your Next.js Web apps to Firebase and
serve them with {{firebase_hosting}}. The {{cli}} respects your Next.js settings and
translates them to Firebase settings with zero or minimal extra configuration on
your part. If your app includes dynamic server-side logic, the {{cli}} deploys that
logic to {{cloud_functions_full}}. The latest supported Next.js version is 13.4.7.
logic to {{cloud_functions_full}}.

<<_includes/_preview-disclaimer.md>>

Expand Down
12 changes: 11 additions & 1 deletion src/frameworks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,21 @@ export async function prepareFrameworks(
name,
support,
docsUrl,
supportedRange,
getValidBuildTargets = GET_DEFAULT_BUILD_TARGETS,
shouldUseDevModeHandle = DEFAULT_SHOULD_USE_DEV_MODE_HANDLE,
} = WebFrameworks[framework];

logger.info(
`\n${frameworksCallToAction(SupportLevelWarnings[support](name), docsUrl, " ")}\n`
`\n${frameworksCallToAction(
SupportLevelWarnings[support](name),
docsUrl,
" ",
name,
results.version,
supportedRange,
results.vite
)}\n`
);

const hostingEmulatorInfo = emulators.find((e) => e.name === Emulators.HOSTING);
Expand Down
3 changes: 3 additions & 0 deletions src/frameworks/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export const enum SupportLevel {
export interface Discovery {
mayWantBackend: boolean;
publicDirectory: string;
version?: string;
vite?: boolean;
}

export interface BuildResult {
Expand Down Expand Up @@ -53,6 +55,7 @@ export type FrameworkContext = {
};

export interface Framework {
supportedRange?: string;
discover: (dir: string) => Promise<Discovery | undefined>;
type: FrameworkType;
name: string;
Expand Down
13 changes: 8 additions & 5 deletions src/frameworks/next/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ import { logger } from "../../logger";
const DEFAULT_BUILD_SCRIPT = ["next build"];
const PUBLIC_DIR = "public";

export const supportedRange = "12 - 14.0";

export const name = "Next.js";
export const support = SupportLevel.Preview;
export const type = FrameworkType.MetaFramework;
Expand All @@ -90,9 +92,10 @@ function getReactVersion(cwd: string): string | undefined {
*/
export async function discover(dir: string) {
if (!(await pathExists(join(dir, "package.json")))) return;
if (!(await pathExists("next.config.js")) && !getNextVersion(dir)) return;
const version = getNextVersion(dir);
if (!(await pathExists("next.config.js")) && !version) return;

return { mayWantBackend: true, publicDirectory: join(dir, PUBLIC_DIR) };
return { mayWantBackend: true, publicDirectory: join(dir, PUBLIC_DIR), version };
}

/**
Expand Down Expand Up @@ -317,9 +320,9 @@ export async function init(setup: any, config: any) {
choices: ["JavaScript", "TypeScript"],
});
execSync(
`npx --yes create-next-app@latest -e hello-world ${setup.hosting.source} --use-npm ${
language === "TypeScript" ? "--ts" : "--js"
}`,
`npx --yes create-next-app@"${supportedRange}" -e hello-world ${
setup.hosting.source
} --use-npm ${language === "TypeScript" ? "--ts" : "--js"}`,
{ stdio: "inherit", cwd: config.projectDir }
);
}
Expand Down
9 changes: 5 additions & 4 deletions src/frameworks/nuxt/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { getNuxtVersion } from "./utils";
export const name = "Nuxt";
export const support = SupportLevel.Experimental;
export const type = FrameworkType.Toolchain;
export const supportedRange = "3";

import { nuxtConfigFilesExist } from "./utils";
import type { NuxtOptions } from "./interfaces";
Expand All @@ -27,16 +28,16 @@ export async function discover(dir: string) {

const anyConfigFileExists = await nuxtConfigFilesExist(dir);

const nuxtVersion = getNuxtVersion(dir);
if (!anyConfigFileExists && !nuxtVersion) return;
if (nuxtVersion && lt(nuxtVersion, "3.0.0-0")) return;
const version = getNuxtVersion(dir);
if (!anyConfigFileExists && !version) return;
if (version && lt(version, "3.0.0-0")) return;

const {
dir: { public: publicDirectory },
ssr: mayWantBackend,
} = await getConfig(dir);

return { publicDirectory, mayWantBackend };
return { publicDirectory, mayWantBackend, version };
}

export async function build(cwd: string) {
Expand Down
7 changes: 4 additions & 3 deletions src/frameworks/nuxt2/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { spawn } from "cross-spawn";
export const name = "Nuxt";
export const support = SupportLevel.Experimental;
export const type = FrameworkType.MetaFramework;
export const supportedRange = "2";

async function getAndLoadNuxt(options: { rootDir: string; for: string }) {
const nuxt = await relativeRequire(options.rootDir, "nuxt/dist/nuxt.js");
Expand All @@ -27,10 +28,10 @@ async function getAndLoadNuxt(options: { rootDir: string; for: string }) {
*/
export async function discover(rootDir: string) {
if (!(await pathExists(join(rootDir, "package.json")))) return;
const nuxtVersion = getNuxtVersion(rootDir);
if (!nuxtVersion || (nuxtVersion && gte(nuxtVersion, "3.0.0-0"))) return;
const version = getNuxtVersion(rootDir);
if (!version || (version && gte(version, "3.0.0-0"))) return;
const { app } = await getAndLoadNuxt({ rootDir, for: "build" });
return { mayWantBackend: true, publicDirectory: app.options.dir.static };
return { mayWantBackend: true, publicDirectory: app.options.dir.static, version };
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/frameworks/sveltekit/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ export const name = "SvelteKit";
export const support = SupportLevel.Experimental;
export const type = FrameworkType.MetaFramework;
export const discover = viteDiscoverWithNpmDependency("@sveltejs/kit");
export { getDevModeHandle } from "../vite";

export { getDevModeHandle, supportedRange } from "../vite";

export async function build(root: string) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sveltekit should probably have it's own supported range

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed but maybe after 2.0 is released? It seems like it won't be released anytime soon: https://github.com/sveltejs/kit/milestone/5

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough

const config = await getConfig(root);
Expand Down
40 changes: 28 additions & 12 deletions src/frameworks/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { readFile } from "fs/promises";
import { IncomingMessage, request as httpRequest, ServerResponse, Agent } from "http";
import { sync as spawnSync } from "cross-spawn";
import * as clc from "colorette";
import { satisfies as semverSatisfied } from "semver";

import { logger } from "../logger";
import { FirebaseError } from "../error";
Expand Down Expand Up @@ -360,21 +361,36 @@ export function relativeRequire(dir: string, mod: string) {
}
}

export function conjoinOptions(
opts: any[],
conjunction: string = "and",
separator: string = ","
): string | undefined {
if (!opts.length) return;
if (opts.length === 1) return opts[0].toString();
if (opts.length === 2) return `${opts[0].toString()} ${conjunction} ${opts[1].toString()}`;
const lastElement = opts.slice(-1)[0].toString();
const allButLast = opts.slice(0, -1).map((it) => it.toString());
export function conjoinOptions(_opts: any[], conjunction = "and", separator = ","): string {
if (!_opts.length) return "";
const opts: string[] = _opts.map((it) => it.toString().trim());
if (opts.length === 1) return opts[0];
if (opts.length === 2) return `${opts[0]} ${conjunction} ${opts[1]}`;
const lastElement = opts.slice(-1)[0];
const allButLast = opts.slice(0, -1);
return `${allButLast.join(`${separator} `)}${separator} ${conjunction} ${lastElement}`;
}

export function frameworksCallToAction(message: string, docsUrl = DEFAULT_DOCS_URL, prefix = "") {
return `${prefix}${message}
export function frameworksCallToAction(
message: string,
docsUrl = DEFAULT_DOCS_URL,
prefix = "",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe prefix could be handled internally in the function based on the params? e.g.:

const prefix = framework ? "   " : "";

just thinking about reducing the parameters here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll refactor this in a follow on

framework?: string,
version?: string,
supportedRange?: string,
vite = false
): string {
return `${prefix}${message}${
framework && supportedRange && (!version || !semverSatisfied(version, supportedRange))
? clc.yellow(
`\n${prefix}The integration is known to work with ${
vite ? "Vite" : framework
} version ${clc.italic(
conjoinOptions(supportedRange.split("||"))
)}. You may encounter errors.`
)
: ``
}

${prefix}${clc.bold("Documentation:")} ${docsUrl}
${prefix}${clc.bold("File a bug:")} ${FILE_BUG_URL}
Expand Down
26 changes: 20 additions & 6 deletions src/frameworks/vite/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
export const name = "Vite";
export const support = SupportLevel.Experimental;
export const type = FrameworkType.Toolchain;
export const supportedRange = "3 - 5";

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

Expand All @@ -32,10 +33,13 @@ export async function init(setup: any, config: any, baseTemplate: string = "vani
{ name: "TypeScript", value: `${baseTemplate}-ts` },
],
});
execSync(`npm create vite@latest ${setup.hosting.source} --yes -- --template ${template}`, {
stdio: "inherit",
cwd: config.projectDir,
});
execSync(
`npm create vite@"${supportedRange}" ${setup.hosting.source} --yes -- --template ${template}`,
{
stdio: "inherit",
cwd: config.projectDir,
}
);
execSync(`npm install`, { stdio: "inherit", cwd: join(config.projectDir, setup.hosting.source) });
}

Expand All @@ -56,11 +60,21 @@ export async function discover(dir: string, plugin?: string, npmDependency?: str
pathExists(join(dir, "vite.config.ts")),
]);
const anyConfigFileExists = configFilesExist.some((it) => it);
if (!anyConfigFileExists && !findDependency("vite", { cwd: dir, depth, omitDev: false })) return;
const version: string | undefined = findDependency("vite", {
cwd: dir,
depth,
omitDev: false,
})?.version;
if (!anyConfigFileExists && !version) return;
if (npmDependency && !additionalDep) return;
const { appType, publicDir: publicDirectory, plugins } = await getConfig(dir);
if (plugin && !plugins.find(({ name }) => name === plugin)) return;
return { mayWantBackend: appType !== "spa", publicDirectory };
return {
mayWantBackend: appType !== "spa",
publicDirectory,
version,
vite: true,
};
}

export async function build(root: string) {
Expand Down
1 change: 1 addition & 0 deletions src/test/frameworks/angular/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe("Angular", () => {
expect(await discover(cwd)).to.deep.equal({
mayWantBackend: true,
publicDirectory: join(cwd, "src", "assets"),
version: undefined,
});
});
});
Expand Down
2 changes: 2 additions & 0 deletions src/test/frameworks/astro/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ describe("Astro", () => {
expect(await discover(cwd)).to.deep.equal({
mayWantBackend: false,
publicDirectory: publicDir,
version: "2.2.2",
});
});

Expand Down Expand Up @@ -84,6 +85,7 @@ describe("Astro", () => {
expect(await discover(cwd)).to.deep.equal({
mayWantBackend: true,
publicDirectory: publicDir,
version: "2.2.2",
});
});
});
Expand Down
2 changes: 2 additions & 0 deletions src/test/frameworks/nuxt/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe("Nuxt 2 utils", () => {
expect(await discoverNuxt2(discoverNuxtDir)).to.deep.equal({
mayWantBackend: true,
publicDirectory: "static",
version: "2.15.8",
});
});

Expand Down Expand Up @@ -75,6 +76,7 @@ describe("Nuxt 2 utils", () => {
expect(await discoverNuxt3(discoverNuxtDir)).to.deep.equal({
mayWantBackend: true,
publicDirectory: "public",
version: "3.0.0",
});
});
});
Expand Down
Loading
Loading