Skip to content

Commit

Permalink
cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
manuel3108 committed Feb 2, 2025
1 parent de838ab commit 2815a8c
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 34 deletions.
27 changes: 24 additions & 3 deletions community-addon-template/tests/setup/suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@ import path from 'node:path';
import { execSync } from 'node:child_process';
import * as vitest from 'vitest';
import { installAddon, type AddonMap, type OptionMap } from 'sv';
import { createProject, startPreview, type CreateProject, type ProjectVariant } from 'sv/testing';
import {
addPnpmBuildDependendencies,
createProject,
startPreview,
type CreateProject,
type ProjectVariant
} from 'sv/testing';
import { chromium, type Browser, type Page } from '@playwright/test';
import { fileURLToPath } from 'node:url';

Expand Down Expand Up @@ -40,9 +46,18 @@ export function setupTest<Addons extends AddonMap>(addons: Addons) {
// creates a pnpm workspace in each addon dir
fs.writeFileSync(
path.resolve(cwd, testName, 'pnpm-workspace.yaml'),
`packages:\n - '**/*'`,
"packages:\n - '**/*'",
'utf8'
);

// creates a barebones package.json in each addon dir
fs.writeFileSync(
path.resolve(cwd, testName, 'package.json'),
JSON.stringify({
name: `${testName}-workspace-root`,
private: true
})
);
});

// runs before each test case
Expand All @@ -57,7 +72,13 @@ export function setupTest<Addons extends AddonMap>(addons: Addons) {
fs.writeFileSync(metaPath, JSON.stringify({ variant, options }, null, '\t'), 'utf8');

// run addon
await installAddon({ cwd, addons, options, packageManager: 'pnpm' });
const { pnpmBuildDependencies } = await installAddon({
cwd,
addons,
options,
packageManager: 'pnpm'
});
addPnpmBuildDependendencies(cwd, 'pnpm', ['esbuild', ...pnpmBuildDependencies]);

return cwd;
};
Expand Down
6 changes: 3 additions & 3 deletions packages/addons/_tests/_setup/suite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { installAddon, type AddonMap, type OptionMap } from 'sv';
import {
createProject,
startPreview,
allowExecutingPostinstallScripts,
addPnpmBuildDependendencies,
type CreateProject,
type ProjectVariant
} from 'sv/testing';
Expand Down Expand Up @@ -69,13 +69,13 @@ export function setupTest<Addons extends AddonMap>(addons: Addons) {
fs.writeFileSync(metaPath, JSON.stringify({ variant, options }, null, '\t'), 'utf8');

// run addon
const { allowedPostinstallScripts } = await installAddon({
const { pnpmBuildDependencies } = await installAddon({
cwd,
addons,
options,
packageManager: 'pnpm'
});
allowExecutingPostinstallScripts(cwd, 'pnpm', ['esbuild', ...allowedPostinstallScripts]);
addPnpmBuildDependendencies(cwd, 'pnpm', ['esbuild', ...pnpmBuildDependencies]);

return cwd;
};
Expand Down
19 changes: 9 additions & 10 deletions packages/cli/commands/add/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { createWorkspace } from './workspace.ts';
import { formatFiles, getHighlighter } from './utils.ts';
import { Directive, downloadPackage, getPackageJSON } from './fetch-packages.ts';
import {
allowExecutingPostinstallScripts,
addPnpmBuildDependendencies,
installDependencies,
packageManagerPrompt
} from '../../utils/package-manager.ts';
Expand Down Expand Up @@ -437,13 +437,12 @@ export async function runAddCommand(
const details = officialDetails.concat(commDetails);

const addonMap: AddonMap = Object.assign({}, ...details.map((a) => ({ [a.id]: a })));
const { filesToFormat, allowedPostinstallScripts: allowedAddonPostinstallScripts } =
await applyAddons({
workspace,
addonSetupResults,
addons: addonMap,
options: official
});
const { filesToFormat, pnpmBuildDependencies: addonPnpmBuildDependencies } = await applyAddons({
workspace,
addonSetupResults,
addons: addonMap,
options: official
});

p.log.success('Successfully setup add-ons');

Expand All @@ -455,9 +454,9 @@ export async function runAddCommand(
if (packageManager) {
workspace.packageManager = packageManager;

allowExecutingPostinstallScripts(workspace.cwd, packageManager, [
addPnpmBuildDependendencies(workspace.cwd, packageManager, [
'esbuild',
...allowedAddonPostinstallScripts
...addonPnpmBuildDependencies
]);

await installDependencies(packageManager, options.cwd);
Expand Down
4 changes: 2 additions & 2 deletions packages/cli/commands/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import * as common from '../utils/common.ts';
import { runAddCommand } from './add/index.ts';
import { detectSync, resolveCommand, type AgentName } from 'package-manager-detector';
import {
allowExecutingPostinstallScripts,
addPnpmBuildDependendencies,
getUserAgent,
installDependencies,
packageManagerPrompt
Expand Down Expand Up @@ -165,7 +165,7 @@ async function createProject(cwd: ProjectPath, options: Options) {
let addOnNextSteps: string | undefined;
const installDeps = async () => {
packageManager = await packageManagerPrompt(projectPath);
allowExecutingPostinstallScripts(projectPath, packageManager, ['esbuild']);
addPnpmBuildDependendencies(projectPath, packageManager, ['esbuild']);
if (packageManager) await installDependencies(packageManager, projectPath);
};

Expand Down
18 changes: 9 additions & 9 deletions packages/cli/lib/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,30 +53,30 @@ export async function applyAddons({
options
}: ApplyAddonOptions): Promise<{
filesToFormat: string[];
allowedPostinstallScripts: string[];
pnpmBuildDependencies: string[];
}> {
const filesToFormat = new Set<string>();
const allowedPostinstallScripts: string[] = [];
const allPnpmBuildDependencies: string[] = [];

const mapped = Object.entries(addons).map(([, addon]) => addon);
const ordered = orderAddons(mapped, addonSetupResults);

for (const addon of ordered) {
workspace = createWorkspace({ ...workspace, options: options[addon.id] });

const { files, allowPostinstallScripts } = await runAddon({
const { files, pnpmBuildDependencies } = await runAddon({
workspace,
addon,
multiple: ordered.length > 1
});

files.forEach((f) => filesToFormat.add(f));
allowPostinstallScripts.forEach((s) => allowedPostinstallScripts.push(s));
pnpmBuildDependencies.forEach((s) => allPnpmBuildDependencies.push(s));
}

return {
filesToFormat: Array.from(filesToFormat),
allowedPostinstallScripts
pnpmBuildDependencies: allPnpmBuildDependencies
};
}

Expand Down Expand Up @@ -116,7 +116,7 @@ async function runAddon({ addon, multiple, workspace }: RunAddon) {
}

const dependencies: Array<{ pkg: string; version: string; dev: boolean }> = [];
const allowPostinstallScripts: string[] = [];
const pnpmBuildDependencies: string[] = [];
const sv: SvApi = {
file: (path, content) => {
try {
Expand Down Expand Up @@ -165,8 +165,8 @@ async function runAddon({ addon, multiple, workspace }: RunAddon) {
devDependency: (pkg, version) => {
dependencies.push({ pkg, version, dev: true });
},
allowPostinstallScript: (pkg) => {
allowPostinstallScripts.push(pkg);
pnpmBuildDependendency: (pkg) => {
pnpmBuildDependencies.push(pkg);
}
};
await addon.run({ ...workspace, sv });
Expand All @@ -176,7 +176,7 @@ async function runAddon({ addon, multiple, workspace }: RunAddon) {

return {
files: Array.from(files),
allowPostinstallScripts
pnpmBuildDependencies
};
}

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/lib/testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import degit from 'degit';
import { exec } from 'tinyexec';
import { create } from '@sveltejs/create';
import pstree, { type PS } from 'ps-tree';
import { allowExecutingPostinstallScripts } from '../utils/package-manager.ts';
import { addPnpmBuildDependendencies } from '../utils/package-manager.ts';

export { allowExecutingPostinstallScripts };
export { addPnpmBuildDependendencies };
export type ProjectVariant = 'kit-js' | 'kit-ts' | 'vite-js' | 'vite-ts';

const TEMPLATES_DIR = '.templates';
Expand Down
9 changes: 5 additions & 4 deletions packages/cli/utils/package-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,31 +72,32 @@ export function getUserAgent(): AgentName | undefined {
return AGENTS.includes(name) ? name : undefined;
}

export function allowExecutingPostinstallScripts(
export function addPnpmBuildDependendencies(
cwd: string,
packageManager: AgentName | null | undefined,
allowedPackages: string[]
) {
// currently we only need to explicitly allow running postinstall
// scripts for pnpm. It's possible that this sets precedence for
// other package managers tho, therefore this has been extracted here.
// other package managers are currently not affected by this change
if (!packageManager || packageManager !== 'pnpm') return;

// find the workspace root
const pnpmWorkspacePath = find.up('pnpm-workspace.yaml', { cwd });
if (!pnpmWorkspacePath) return;

// load the package.json
const pkgPath = path.join(path.dirname(pnpmWorkspacePath), 'package.json');
const content = fs.readFileSync(pkgPath, 'utf-8');
const { data, generateCode } = parseJson(content);

// add the packages where we want the postinstall scripts to be run
data.pnpm ??= {};
data.pnpm.onlyBuiltDependencies ??= [];
for (const allowedPackage of allowedPackages) {
if (data.pnpm.onlyBuiltDependencies.includes(allowedPackage)) continue;
data.pnpm.onlyBuiltDependencies.push(allowedPackage);
}

// save the updated package.json
const newContent = generateCode();
fs.writeFileSync(pkgPath, newContent);
}
2 changes: 1 addition & 1 deletion packages/core/addon/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export type Scripts<Args extends OptionDefinition> = {
};

export type SvApi = {
allowPostinstallScript: (pkg: string) => void;
pnpmBuildDependendency: (pkg: string) => void;
dependency: (pkg: string, version: string) => void;
devDependency: (pkg: string, version: string) => void;
execute: (args: string[], stdio: 'inherit' | 'pipe') => Promise<void>;
Expand Down

0 comments on commit 2815a8c

Please sign in to comment.