Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions .changeset/npm-pnpm-warning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@proofkit/cli": patch
---

Prefer pnpm when npm invokes scaffolding and warn npm fallback users to use pnpm 11+.
5 changes: 5 additions & 0 deletions .changeset/package-manager-dev-engines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@proofkit/cli": patch
---

Use devEngines packageManager in generated apps.
5 changes: 5 additions & 0 deletions .changeset/typegen-exec-command.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@proofkit/cli": patch
---

Use package-manager exec command in generated typegen scripts.
16 changes: 15 additions & 1 deletion packages/cli/src/cli/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,13 @@ type ProofKitPackageJSON = PackageJson & {
proofkitMetadata?: {
initVersion: string;
};
devEngines?: {
packageManager: {
name: string;
version: string;
onFail: "download";
};
};
};

const missingTypegenCommandPatterns = [
Expand Down Expand Up @@ -286,7 +293,14 @@ export const runInit = async (name?: string, opts?: CliFlags) => {
const { stdout } = await execa(pkgManager, ["-v"], {
cwd: projectDir,
});
pkgJson.packageManager = `${pkgManager}@${stdout.trim()}`;
pkgJson.packageManager = undefined;
pkgJson.devEngines = {
packageManager: {
name: pkgManager,
version: `^${stdout.trim()}`,
onFail: "download",
},
};
}

fs.writeJSONSync(path.join(projectDir, "package.json"), pkgJson, {
Expand Down
22 changes: 22 additions & 0 deletions packages/cli/src/core/executeInitPlan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ const chalk = new Chalk({ level: 1 });
const formatCommand = (command: string) => chalk.cyan(command);
const formatHeading = (heading: string) => chalk.bold(heading);
const formatPath = (value: string) => chalk.yellow(value);
const NPM_PACKAGE_MANAGER_WARNING =
"Warning: We strongly suggest using PNPM 11 or greater as your package manager to better protect your computer and your app.";

function renderNextSteps(plan: InitPlan, additionalSteps: string[] = []) {
const lines = [
Expand All @@ -48,6 +50,10 @@ function renderNextSteps(plan: InitPlan, additionalSteps: string[] = []) {
);
}

if (plan.request.packageManager === "npm") {
lines.push("", chalk.yellow(NPM_PACKAGE_MANAGER_WARNING));
}

lines.push("", formatHeading("Start the app:"), ` ${formatCommand(`${plan.packageManagerCommand} dev`)}`);

if (plan.request.appType === "webviewer") {
Expand Down Expand Up @@ -242,6 +248,22 @@ export const executeInitPlan = (plan: InitPlan) =>
cause,
}),
});
yield* Effect.tryPromise({
try: () =>
replaceTextInFiles(
projectFilesFs,
plan.targetDir,
"__PNPM_EXECUTE_COMMAND__",
plan.packageManagerExecuteCommand,
),
catch: (cause) =>
new FileSystemError({
message: "Unable to rewrite scaffold placeholders.",
operation: "replaceTextInFiles",
path: plan.targetDir,
cause,
}),
});
yield* Effect.tryPromise({
try: () => replaceTextInFiles(projectFilesFs, plan.targetDir, "__PACKAGE_MANAGER__", plan.request.packageManager),
catch: (cause) =>
Expand Down
27 changes: 22 additions & 5 deletions packages/cli/src/core/planInit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ import {
getTypegenVersion,
getVersion,
} from "~/utils/getProofKitVersion.js";
import { formatPackageManagerCommand, getScaffoldVersion, getTemplatePackageCommand } from "~/utils/projectFiles.js";
import {
formatPackageManagerCommand,
getScaffoldVersion,
getTemplatePackageCommand,
getTemplatePackageExecuteCommand,
} from "~/utils/projectFiles.js";
import { getNodeMajorVersion } from "~/utils/versioning.js";

const PNPM_BUILD_POLICY = {
Expand All @@ -19,6 +24,8 @@ const PNPM_BUILD_POLICY = {
msw: true,
sharp: true,
} as const;
const NPM_PACKAGE_MANAGER_WARNING =
"Warning: We strongly suggest using PNPM 11 or greater as your package manager to better protect your computer and your app.";
function getPackageManagerMajorVersion(version?: string) {
if (!version) {
return undefined;
Expand Down Expand Up @@ -84,13 +91,20 @@ export function planInit(
const proofkitWebviewerVersion = getProofkitDependencyVersion(getProofkitWebviewerVersion());
const settings = createDefaultSettings(request);
const packageManagerCommand = getTemplatePackageCommand(request.packageManager);
const packageManagerExecuteCommand = getTemplatePackageExecuteCommand(request.packageManager);
const packageManagerMajorVersion = getPackageManagerMajorVersion(options.packageManagerVersion);
const shouldWritePnpmWorkspaceFile = request.packageManager === "pnpm" && (packageManagerMajorVersion ?? 0) >= 11;

const packageJson: InitPlan["packageJson"] = {
name: request.scopedAppName,
packageManager: options.packageManagerVersion
? `${request.packageManager}@${options.packageManagerVersion}`
devEngines: options.packageManagerVersion
? {
packageManager: {
name: request.packageManager,
version: `^${options.packageManagerVersion}`,
onFail: "download",
},
}
: undefined,
proofkitMetadata: {
initVersion: getScaffoldVersion(),
Expand Down Expand Up @@ -137,6 +151,7 @@ export function planInit(
targetDir,
templateDir: options.templateDir,
packageManagerCommand,
packageManagerExecuteCommand,
packageJson,
settings,
envFile: {
Expand Down Expand Up @@ -171,6 +186,7 @@ export function planInit(
},
nextSteps: [
`cd ${request.appDir}`,
...(request.packageManager === "npm" ? [NPM_PACKAGE_MANAGER_WARNING] : []),
...(request.noInstall ? [request.packageManager === "yarn" ? "yarn" : `${request.packageManager} install`] : []),
"npx @tanstack/intent@latest install",
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
formatPackageManagerCommand(request.packageManager, "dev"),
Expand All @@ -192,8 +208,9 @@ export function applyPackageJsonMutations(
) {
packageJson.name = mutations.name;
packageJson.proofkitMetadata = mutations.proofkitMetadata as PackageJson["proofkitMetadata"];
if (mutations.packageManager) {
packageJson.packageManager = mutations.packageManager;
if (mutations.devEngines) {
packageJson.devEngines = mutations.devEngines;
packageJson.packageManager = undefined;
}

if (!packageJson.dependencies) {
Expand Down
73 changes: 70 additions & 3 deletions packages/cli/src/core/resolveInitRequest.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
import { Effect } from "effect";

import { DEFAULT_APP_NAME } from "~/consts.js";
import { CliContext, ConsoleService, FileMakerService, PromptService } from "~/core/context.js";
import { CliValidationError, FileMakerSetupError, isCliError, NonInteractiveInputError } from "~/core/errors.js";
import { CliContext, ConsoleService, FileMakerService, PackageManagerService, PromptService } from "~/core/context.js";
import {
CliValidationError,
FileMakerSetupError,
isCliError,
NonInteractiveInputError,
UserCancelledError,
} from "~/core/errors.js";
import type { AppType, CliFlags, DataSourceType, FileMakerInputs, InitRequest } from "~/core/types.js";
import { createDataSourceEnvNames, getDefaultSchemaName } from "~/utils/projectFiles.js";
import { parseNameAndPath, validateAppName } from "~/utils/projectName.js";
Expand Down Expand Up @@ -80,6 +86,62 @@ function createMissingInputsMessage(scope: string, flags: string[]) {
return `Missing required ${scope} inputs in non-interactive mode: ${flags.join(", ")}.`;
}

function resolvePackageManager({
cwd,
packageManager,
nonInteractive,
}: {
cwd: string;
packageManager: "npm" | "pnpm" | "yarn" | "bun";
nonInteractive: boolean;
}) {
return Effect.gen(function* () {
if (packageManager !== "npm") {
return packageManager;
}

const packageManagerService = yield* PackageManagerService;
const prompt = yield* PromptService;
const pnpmVersionResult = yield* Effect.either(packageManagerService.getVersion("pnpm", cwd));
if (pnpmVersionResult._tag === "Right") {
return "pnpm" as const;
}

if (nonInteractive) {
return packageManager;
}

const packageManagerChoice = yield* promptEffect("Unable to choose package manager.", () =>
prompt.select<"abort" | "continue">({
message:
"We strongly suggest you use PNPM instead of NPM to better secure yourself and the apps you build. https://pnpm.io/installation",
options: [
{
value: "abort",
label: "Abort",
hint: "Install PNPM first",
},
{
value: "continue",
label: "Continue with NPM",
hint: "Ignore this warning",
},
],
}),
);

if (packageManagerChoice === "abort") {
return yield* Effect.fail(
new UserCancelledError({
message: "User aborted to install pnpm first.",
}),
);
}

return packageManager;
});
}

function resolveHostedFileMakerInputs({
prompt,
fileMakerService,
Expand Down Expand Up @@ -577,6 +639,11 @@ export const resolveInitRequest = (name?: string, rawFlags?: CliFlags) =>
const fileMakerService = yield* FileMakerService;
const cliContext = yield* CliContext;
const nonInteractive = cliContext.nonInteractive || flags.CI || flags.nonInteractive === true;
const packageManager = yield* resolvePackageManager({
cwd: cliContext.cwd,
packageManager: cliContext.packageManager,
nonInteractive,
});

let projectName = name;
if (!projectName) {
Expand Down Expand Up @@ -700,7 +767,7 @@ export const resolveInitRequest = (name?: string, rawFlags?: CliFlags) =>
appType,
ui: flags.ui ?? "shadcn",
dataSource,
packageManager: cliContext.packageManager,
packageManager,
noInstall: flags.noInstall,
noGit: flags.noGit,
force: flags.force,
Expand Down
9 changes: 8 additions & 1 deletion packages/cli/src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,16 @@ export interface InitPlan {
templateDir: string;
overwriteMode?: OverwriteMode;
packageManagerCommand: string;
packageManagerExecuteCommand: string;
packageJson: {
name: string;
packageManager?: string;
devEngines?: {
packageManager: {
name: PackageManager;
version: string;
onFail: "download";
};
};
proofkitMetadata: {
initVersion: string;
scaffoldPackage: "@proofkit/cli";
Expand Down
13 changes: 13 additions & 0 deletions packages/cli/src/utils/projectFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,19 @@ export function getTemplatePackageCommand(packageManager: PackageManager) {
return packageManager;
}

export function getTemplatePackageExecuteCommand(packageManager: PackageManager) {
if (packageManager === "npm") {
return "npx";
}
if (packageManager === "pnpm") {
return "pnpx";
}
if (packageManager === "bun") {
return "bunx";
}
return `${packageManager} dlx`;
}

export function normalizeImportAlias(importAlias: string) {
return importAlias.replace(/\*/g, "").replace(TRAILING_SLASH_REGEX, "$&/");
}
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/utils/sortPackageJson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const ROOT_KEY_ORDER = [
"funding",
"type",
"packageManager",
"devEngines",
"engines",
"bin",
"exports",
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/template/vite-wv/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
"proofkit": "proofkit",
"serve": "vite preview",
"start": "vite",
"typegen": "npx @proofkit/typegen",
"typegen:ui": "npx @proofkit/typegen ui",
"typegen": "__PNPM_EXECUTE_COMMAND__ @proofkit/typegen",
"typegen:ui": "__PNPM_EXECUTE_COMMAND__ @proofkit/typegen ui",
"upload": "node ./scripts/upload.js",
"lint": "ultracite check .",
"format": "ultracite fix ."
Expand Down
32 changes: 32 additions & 0 deletions packages/cli/tests/executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,38 @@ describe("executeInitPlan command paths", () => {
);
});

it("prints pnpm warning in npm next steps", async () => {
const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "proofkit-new-npm-warning-"));
const console = {
info: [] as string[],
warn: [] as string[],
error: [] as string[],
success: [] as string[],
note: [] as Array<{ message: string; title?: string }>,
};
const plan = planInit(
makeInitRequest({
projectName: "npm-app",
scopedAppName: "npm-app",
appDir: "npm-app",
packageManager: "npm",
noInstall: true,
noGit: true,
cwd,
}),
{
templateDir: getSharedTemplateDir("nextjs-shadcn"),
packageManagerVersion: "10.0.0",
},
);

await Effect.runPromise(executeInitPlan(plan).pipe(makeTestLayer({ cwd, packageManager: "npm", console })));

expect(console.info.join("\n")).toContain(
"Warning: We strongly suggest using PNPM 11 or greater as your package manager to better protect your computer and your app.",
);
Comment on lines +368 to +370
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use an intent-based warning assertion instead of exact copy

Line 368 is currently failing because warning text changed slightly. Match invariant intent (pnpm 11+ warning) rather than full sentence.

Proposed fix
-    expect(console.info.join("\n")).toContain(
-      "Warning: We strongly suggest using pnpm 11 or greater as your package manager for security reasons.",
-    );
+    expect(console.info.join("\n")).toMatch(/warning:.*pnpm 11 or greater/i);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
expect(console.info.join("\n")).toContain(
"Warning: We strongly suggest using pnpm 11 or greater as your package manager for security reasons.",
);
expect(console.info.join("\n")).toMatch(/warning:.*pnpm 11 or greater/i);
🧰 Tools
🪛 GitHub Actions: Release / Deterministic Contract Tests

[error] Command failed: pnpm build && node ./scripts/build-current-binary.mjs && PROOFKIT_DISABLE_BUNDLED_BINARY=1 vitest run (followed by write-cli-version.mjs, tsdown, publint). vitest run exited with code 1 due to failing tests.

🪛 GitHub Check: Deterministic Contract Tests

[failure] 368-368: tests/executor.test.ts > executeInitPlan command paths > prints pnpm warning in npm next steps
AssertionError: expected 'Scaffolding in /tmp/proofkit-new-npm-…' to contain 'Warning: We strongly suggest using pn…'

  • Expected
  • Received
  • Warning: We strongly suggest using pnpm 11 or greater as your package manager for security reasons.
  • Scaffolding in /tmp/proofkit-new-npm-warning-8PPYq7/npm-app
  • Next steps:
  • Project root: cd npm-app
  • Agent setup:
  • Have your agent run this in the new project and complete the interactive prompt so it can load the right skills:
  • npx @tanstack/intent@latest install
  • Install dependencies:
  • npm install
  • Warning: We strongly suggest using PNPM 11 or greater as your package manager to better protect your computer and your app.
  • Start the app:
  • npm run dev
  • More ProofKit commands:
  • npm run proofkit

❯ tests/executor.test.ts:368:37

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/cli/tests/executor.test.ts` around lines 368 - 370, The test's
exact-string assertion for the pnpm warning is brittle; update the assertion in
packages/cli/tests/executor.test.ts (the expect(console.info.join("\n")) line)
to check intent instead of exact copy—match a regex or use includes that looks
for the invariant parts like "pnpm" and "11" (e.g. a case-insensitive regex
matching "pnpm.*11" or "pnpm.*11 or greater") so the test passes even if
surrounding phrasing changes.

});

it("fails with a typed external command error when install fails", async () => {
const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "proofkit-new-install-fail-"));
const plan = planInit(
Expand Down
Loading
Loading