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

C3: add vite+react templates with tests #8013

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
5 changes: 5 additions & 0 deletions .changeset/nine-states-call.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"create-cloudflare": patch
---

add experimental React templates using the Cloudflare Vite plugin
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@
"@types/react-transition-group>@types/react": "^18",
"@cloudflare/elements>@types/react": "^18",
"capnpc-ts>typescript": "4.2.4",
"@types/node": "$@types/node"
"@types/node": "$@types/node",
"vitest>vite": "^5.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this > magic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is standard pnpm magic, which means override only vite version but only those that are dependencies of vitest.

},
"patchedDependencies": {
"@cloudflare/[email protected]": "patches/@[email protected]",
Expand Down
47 changes: 35 additions & 12 deletions packages/create-cloudflare/e2e-tests/frameworks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,26 @@ function getFrameworkTests(opts: {
},
flags: ["--style", "sass"],
},
react: {
promptHandlers: [
{
matcher: /Select a variant:/,
input: [keys.enter],
},
],
unsupportedOSs: ["win32"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current create-vite template doesn't pass on Windows either, so this is not a regression. But at some point we should look into fixing that.

unsupportedPms: ["yarn"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Create-vite basically tries to parse the npm_config_user_agent environment variable - expecting it to look like: yarn/1.22.22 node/20.0.0 etc - to grab the version of yarn.

Because we are running create-cloudflare via pnpx and "faking" the package manager it doesn't work. Something (I think the bit of pnpx/npx that actually creates the child process) is stripping the version so npm_config_user_agent ends up just looking like: yarn (no version).

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think there's any way to fix this?

testCommitMessage: true,
verifyDeploy: {
route: "/",
expectedText: "Vite + React",
},
verifyPreview: {
route: "/",
previewArgs: ["--host=127.0.0.1"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary because by default vite preview is only listening on localhost, which doesn't appear to include 127.0.0.1 (at least on some OSes).

expectedText: "Vite + React",
},
},
gatsby: {
unsupportedPms: ["bun", "pnpm"],
promptHandlers: [
Expand Down Expand Up @@ -641,6 +661,10 @@ describe.concurrent(

test({ experimental }).runIf(shouldRunTest(frameworkId, testConfig))(
frameworkId,
{
retry: TEST_RETRIES,
timeout: testConfig.timeout || TEST_TIMEOUT,
},
async ({ logStream, project }) => {
if (!testConfig.verifyDeploy) {
expect(
Expand Down Expand Up @@ -715,10 +739,6 @@ describe.concurrent(
}
}
},
{
retry: TEST_RETRIES,
timeout: testConfig.timeout || TEST_TIMEOUT,
},
);
});
},
Expand All @@ -742,6 +762,7 @@ const runCli = async (
NO_DEPLOY ? "--no-deploy" : "--deploy",
"--no-open",
"--no-git",
"--no-auto-update",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is probably not really needed but seemed like a safe thing to do to ensure that we are actually running the instance of C3 that we expected.

];

args.push(...argv);
Expand All @@ -765,28 +786,29 @@ const runCli = async (
};

/**
* Either update or create a wrangler.toml to include a `TEST` var.
* Either update or create a wrangler configuration file to include a `TEST` var.
*
* This is rather than having a wrangler.toml in the e2e test's fixture folder,
* This is rather than having a wrangler configuration file in the e2e test's fixture folder,
* which overwrites any that comes from the framework's template.
*/
const addTestVarsToWranglerToml = async (projectPath: string) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe not for this PR, but the name of this variable and all the references to wrangler.toml should be updated to be format agnostic.

const wranglerTomlPath = join(projectPath, "wrangler.toml");
const wranglerJsoncPath = join(projectPath, "wrangler.jsonc");

if (existsSync(wranglerTomlPath)) {
const wranglerToml = readToml(wranglerTomlPath);
// Add a TEST var to the wrangler.toml
wranglerToml.vars ??= {};
(wranglerToml.vars as JsonMap).TEST = "C3_TEST";

writeToml(wranglerTomlPath, wranglerToml);
} else if (existsSync(wranglerJsoncPath)) {
const wranglerJson = readJSON(wranglerJsoncPath);
// Add a TEST var to the wrangler.toml
wranglerJson.vars ??= {};
wranglerJson.vars.TEST = "C3_TEST";
const wranglerJsonc = readJSON(wranglerJsoncPath) as {
vars: Record<string, string>;
};
wranglerJsonc.vars ??= {};
wranglerJsonc.vars.TEST = "C3_TEST";

writeJSON(wranglerJsoncPath, wranglerJson);
writeJSON(wranglerJsoncPath, wranglerJsonc);
}
};

Expand Down Expand Up @@ -838,6 +860,7 @@ const verifyPreviewScript = async (
...(pm === "npm" ? ["--"] : []),
"--port",
`${TEST_PORT}`,
...(verifyPreview.previewArgs ?? []),
],
{
cwd: projectPath,
Expand Down
1 change: 1 addition & 0 deletions packages/create-cloudflare/e2e-tests/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export type RunnerConfig = {
* Specifies whether to run the preview script for the project and assert the response from the specified route.
*/
verifyPreview: null | {
previewArgs?: string[];
route: string;
expectedText: string;
};
Expand Down
13 changes: 8 additions & 5 deletions packages/create-cloudflare/e2e-tests/workers.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { existsSync } from "fs";
import { join } from "path";
import { readJSON, readToml } from "helpers/files";
import { detectPackageManager } from "helpers/packageManagers";
Expand Down Expand Up @@ -177,6 +178,7 @@ describe
const name = testConfig.name ?? testConfig.template;
test({ experimental })(
name,
{ retry: 1, timeout: testConfig.timeout || TEST_TIMEOUT },
async ({ project, logStream }) => {
try {
const deployedUrl = await runCli(
Expand All @@ -197,18 +199,20 @@ describe
const tomlPath = join(project.path, "wrangler.toml");
const jsoncPath = join(project.path, "wrangler.jsonc");

try {
expect(jsoncPath).toExist();
if (existsSync(jsoncPath)) {
const config = readJSON(jsoncPath) as { main?: string };
if (config.main) {
expect(join(project.path, config.main)).toExist();
}
} catch (e) {
expect(tomlPath).toExist();
} else if (existsSync(tomlPath)) {
const config = readToml(tomlPath) as { main?: string };
if (config.main) {
expect(join(project.path, config.main)).toExist();
}
} else {
expect.fail(
`Expected at least one of "${jsoncPath}" or "${tomlPath}" to exist.`,
);
}

const { verifyDeploy, verifyTest } = testConfig;
Expand All @@ -227,7 +231,6 @@ describe
await deleteWorker(project.name);
}
},
{ retry: 1, timeout: testConfig.timeout || TEST_TIMEOUT },
);
});
});
Expand Down
1 change: 1 addition & 0 deletions packages/create-cloudflare/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"@typescript-eslint/parser": "^6.9.0",
"chalk": "^5.2.0",
"command-exists": "^1.2.9",
"comment-json": "^4.2.5",
"cross-spawn": "^7.0.3",
"deepmerge": "^4.3.1",
"degit": "^2.8.4",
Expand Down
14 changes: 7 additions & 7 deletions packages/create-cloudflare/src/helpers/files.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import fs, { existsSync, statSync } from "fs";
import { join } from "path";
import TOML from "@iarna/toml";
import { parse } from "jsonc-parser";
import { parse, stringify } from "comment-json";
import type { JsonMap } from "@iarna/toml";
import type { C3Context } from "types";
import type { C3Context, PackageJson } from "types";

export const copyFile = (path: string, dest: string) => {
try {
Expand Down Expand Up @@ -57,7 +57,7 @@ export const directoryExists = (path: string): boolean => {
}
};

export const readJSON = (path: string) => {
export const readJSON = (path: string): unknown => {
const contents = readFile(path);
return contents ? parse(contents) : contents;
};
Expand All @@ -69,10 +69,10 @@ export const readToml = (path: string) => {

export const writeJSON = (
path: string,
object: object,
object: unknown,
stringifySpace = "\t",
) => {
writeFile(path, JSON.stringify(object, null, stringifySpace));
writeFile(path, stringify(object, null, stringifySpace));
};

export const writeToml = (path: string, object: JsonMap) => {
Expand Down Expand Up @@ -136,8 +136,8 @@ export const usesEslint = (ctx: C3Context): EslintUsageInfo => {
}

try {
const pkgJson = readJSON(`${ctx.project.path}/package.json`);
if (pkgJson.eslintConfig) {
const pkgJson = readJSON(`${ctx.project.path}/package.json`) as PackageJson;
if (pkgJson?.eslintConfig) {
return {
used: true,
configType: "package.json",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const detectPackageManager = () => {
if (process.env.TEST_PM && process.env.TEST_PM_VERSION) {
name = process.env.TEST_PM as PmName;
version = process.env.TEST_PM_VERSION;
process.env.npm_config_user_agent = name;
process.env.npm_config_user_agent = `${name}/${version}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was an attempt to get yarn working with create-vite but something appears to be stripping the version between us running pnpx and create-vite being run.

}

switch (name) {
Expand Down
8 changes: 5 additions & 3 deletions packages/create-cloudflare/src/templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import honoTemplateExperimental from "templates-experimental/hono/c3";
import nextTemplateExperimental from "templates-experimental/next/c3";
import nuxtTemplateExperimental from "templates-experimental/nuxt/c3";
import qwikTemplateExperimental from "templates-experimental/qwik/c3";
import reactTemplateExperimental from "templates-experimental/react/c3";
import remixTemplateExperimental from "templates-experimental/remix/c3";
import solidTemplateExperimental from "templates-experimental/solid/c3";
import svelteTemplateExperimental from "templates-experimental/svelte/c3";
Expand Down Expand Up @@ -177,6 +178,7 @@ export function getFrameworkMap({ experimental = false }): TemplateMap {
next: nextTemplateExperimental,
nuxt: nuxtTemplateExperimental,
qwik: qwikTemplateExperimental,
react: reactTemplateExperimental,
remix: remixTemplateExperimental,
solid: solidTemplateExperimental,
svelte: svelteTemplateExperimental,
Expand Down Expand Up @@ -717,7 +719,7 @@ export const updatePackageName = async (ctx: C3Context) => {
// Update package.json with project name
const placeholderNames = ["<TBD>", "TBD", ""];
const pkgJsonPath = resolve(ctx.project.path, "package.json");
const pkgJson = readJSON(pkgJsonPath);
const pkgJson = readJSON(pkgJsonPath) as PackageJson;

if (!placeholderNames.includes(pkgJson.name)) {
return;
Expand All @@ -741,11 +743,11 @@ export const updatePackageScripts = async (ctx: C3Context) => {
s.start("Updating `package.json` scripts");

const pkgJsonPath = resolve(ctx.project.path, "package.json");
let pkgJson = readJSON(pkgJsonPath);
let pkgJson = readJSON(pkgJsonPath) as PackageJson;

// Run any transformers defined by the template
const transformed = await ctx.template.transformPackageJson(pkgJson, ctx);
pkgJson = deepmerge(pkgJson, transformed);
pkgJson = deepmerge(pkgJson, transformed as PackageJson);

writeJSON(pkgJsonPath, pkgJson);
s.stop(`${brandColor("updated")} ${dim("`package.json`")}`);
Expand Down
28 changes: 23 additions & 5 deletions packages/create-cloudflare/templates-experimental/angular/c3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { readFile, readJSON, writeFile } from "helpers/files";
import { detectPackageManager } from "helpers/packageManagers";
import { installPackages } from "helpers/packages";
import type { TemplateConfig } from "../../src/templates";
import type { C3Context } from "types";
import type { C3Context, PackageJson } from "types";

const { npm } = detectPackageManager();

Expand Down Expand Up @@ -63,10 +63,10 @@ async function updateAppCode() {
// Remove unwanted dependencies
s.start(`Updating package.json`);
const packageJsonPath = resolve("package.json");
const packageManifest = readJSON(packageJsonPath);
const packageManifest = readJSON(packageJsonPath) as PackageJson;

delete packageManifest["dependencies"]["express"];
delete packageManifest["devDependencies"]["@types/express"];
delete packageManifest["dependencies"]?.["express"];
delete packageManifest["devDependencies"]?.["@types/express"];

writeFile(packageJsonPath, JSON.stringify(packageManifest, null, 2));
s.stop(`${brandColor(`updated`)} ${dim(`\`package.json\``)}`);
Expand All @@ -75,7 +75,7 @@ async function updateAppCode() {
function updateAngularJson(ctx: C3Context) {
const s = spinner();
s.start(`Updating angular.json config`);
const angularJson = readJSON(resolve("angular.json"));
const angularJson = readJSON(resolve("angular.json")) as AngularJson;
// Update builder
const architectSection = angularJson.projects[ctx.project.name].architect;
architectSection.build.options.outputPath = "dist";
Expand Down Expand Up @@ -110,3 +110,21 @@ const config: TemplateConfig = {
}),
};
export default config;

type AngularJson = {
projects: Record<
string,
{
architect: {
build: {
options: {
outputPath: string;
outputMode: string;
ssr: Record<string, unknown>;
assets: string[];
};
};
};
}
>;
};
Loading
Loading