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

Reduce create-astro dependencies #8077

Merged
merged 1 commit into from
Aug 14, 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
chore: slim create-astro deps
  • Loading branch information
natemoo-re committed Aug 14, 2023
commit fe5ae2393eab789afd6c65c49920a2a173a8c3d9
5 changes: 5 additions & 0 deletions .changeset/moody-houses-drum.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'create-astro': minor
---

Reduce dependency installation size, swap `execa` for light `node:child_process` wrapper
7 changes: 3 additions & 4 deletions packages/create-astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,14 @@
"//b": "DEPENDENCIES IS FOR UNBUNDLED PACKAGES",
"dependencies": {
"@astrojs/cli-kit": "^0.2.3",
"chai": "^4.3.7",
"execa": "^6.1.0",
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed this!

"giget": "1.0.0",
"mocha": "^9.2.2",
"giget": "^1.1.2",
Copy link
Member Author

Choose a reason for hiding this comment

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

giget was pinned, but that's no longer needed

"node-fetch-native": "^1.2.0",
"which-pm-runs": "^1.1.0"
},
"devDependencies": {
"@types/which-pm-runs": "^1.0.0",
"chai": "^4.3.7",
"mocha": "^9.2.2",
Comment on lines +41 to +42
Copy link
Member Author

Choose a reason for hiding this comment

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

These should have always been dev deps!

"arg": "^5.0.2",
"astro-scripts": "workspace:*",
"strip-ansi": "^7.1.0",
Expand Down
9 changes: 2 additions & 7 deletions packages/create-astro/src/actions/dependencies.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { color } from '@astrojs/cli-kit';
import { execa } from 'execa';
import { shell } from '../shell.js';
import fs from 'node:fs';
import path from 'node:path';
import { error, info, spinner, title } from '../messages.js';
Expand Down Expand Up @@ -49,12 +49,7 @@ export async function dependencies(

async function install({ pkgManager, cwd }: { pkgManager: string; cwd: string }) {
if (pkgManager === 'yarn') await ensureYarnLock({ cwd });
const installExec = execa(pkgManager, ['install'], { cwd });
return new Promise<void>((resolve, reject) => {
setTimeout(() => reject(`Request timed out after 1m 30s`), 90_000);
installExec.on('error', (e) => reject(e));
installExec.on('close', () => resolve());
});
return shell(pkgManager, ['install'], { cwd, timeout: 90_000, stdio: 'ignore' });
}

async function ensureYarnLock({ cwd }: { cwd: string }) {
Expand Down
8 changes: 4 additions & 4 deletions packages/create-astro/src/actions/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import path from 'node:path';
import type { Context } from './context';

import { color } from '@astrojs/cli-kit';
import { execa } from 'execa';
import { shell } from '../shell.js';
import { error, info, spinner, title } from '../messages.js';

export async function git(ctx: Pick<Context, 'cwd' | 'git' | 'yes' | 'prompt' | 'dryRun'>) {
Expand Down Expand Up @@ -45,9 +45,9 @@ export async function git(ctx: Pick<Context, 'cwd' | 'git' | 'yes' | 'prompt' |

async function init({ cwd }: { cwd: string }) {
try {
await execa('git', ['init'], { cwd, stdio: 'ignore' });
await execa('git', ['add', '-A'], { cwd, stdio: 'ignore' });
await execa(
await shell('git', ['init'], { cwd, stdio: 'ignore' });
await shell('git', ['add', '-A'], { cwd, stdio: 'ignore' });
await shell(
'git',
[
'commit',
Expand Down
4 changes: 2 additions & 2 deletions packages/create-astro/src/messages.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint no-console: 'off' */
import { color, say as houston, label, spinner as load } from '@astrojs/cli-kit';
import { align, sleep } from '@astrojs/cli-kit/utils';
import { execa } from 'execa';
import { shell } from './shell.js';
import fetch from 'node-fetch-native';
import { exec } from 'node:child_process';
import stripAnsi from 'strip-ansi';
Expand All @@ -14,7 +14,7 @@ import detectPackageManager from 'which-pm-runs';
async function getRegistry(): Promise<string> {
const packageManager = detectPackageManager()?.name || 'npm';
try {
const { stdout } = await execa(packageManager, ['config', 'get', 'registry']);
const { stdout } = await shell(packageManager, ['config', 'get', 'registry']);
return stdout?.trim()?.replace(/\/$/, '') || 'https://registry.npmjs.org';
} catch (e) {
return 'https://registry.npmjs.org';
Expand Down
44 changes: 44 additions & 0 deletions packages/create-astro/src/shell.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// This is an extremely simplified version of [`execa`](https://github.com/sindresorhus/execa)
// intended to keep our dependency size down
Comment on lines +1 to +2
Copy link
Member Author

Choose a reason for hiding this comment

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

Node built-ins are pretty powerful these days, so execa isn't really needed

Copy link
Member

Choose a reason for hiding this comment

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

Doing this for Astro itself would probably be also great

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed! We can follow-up on that

import type { StdioOptions } from 'node:child_process';
import type { Readable } from 'node:stream';

import { text as textFromStream } from 'node:stream/consumers';
import { spawn } from 'node:child_process';
import { setTimeout as sleep } from 'node:timers/promises';

export interface ExecaOptions {
cwd?: string | URL;
stdio?: StdioOptions;
timeout?: number;
}
export interface Output {
stdout: string;
stderr: string;
exitCode: number;
}
const text = (stream: NodeJS.ReadableStream | Readable | null) => stream ? textFromStream(stream).then(t => t.trimEnd()) : '';

export async function shell(command: string, flags: string[], opts: ExecaOptions = {}): Promise<Output> {
const controller = opts.timeout ? new AbortController() : undefined;
const child = spawn(command, flags, {
cwd: opts.cwd,
shell: true,
stdio: opts.stdio,
signal: controller?.signal
})
const stdout = await text(child.stdout);
const stderr = await text(child.stderr);
if (opts.timeout) {
sleep(opts.timeout).then(() => {
controller!.abort();
throw { stdout, stderr, exitCode: 1 }
})
}
await new Promise((resolve) => child.on('exit', resolve))
const { exitCode } = child;
if (exitCode !== 0) {
throw { stdout, stderr, exitCode };
}
return { stdout, stderr, exitCode }
}
1 change: 1 addition & 0 deletions packages/create-astro/test/fixtures/not-empty/git.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
47 changes: 28 additions & 19 deletions packages/create-astro/test/git.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { expect } from 'chai';

import { execa } from 'execa';
import fs from 'node:fs';
import { mkdir, writeFile } from 'node:fs/promises';
import { rmSync } from 'node:fs';

import { git } from '../dist/index.js';
import { setup } from './utils.js';
Expand All @@ -16,22 +15,6 @@ describe('git', () => {
expect(fixture.hasMessage('Skipping Git initialization')).to.be.true;
});

it('already initialized', async () => {
const context = {
git: true,
cwd: './test/fixtures/not-empty',
dryRun: true,
prompt: () => ({ git: false }),
};
await execa('git', ['init'], { cwd: './test/fixtures/not-empty' });
await git(context);

expect(fixture.hasMessage('Git has already been initialized')).to.be.true;

// Cleanup
fs.rmSync('./test/fixtures/not-empty/.git', { recursive: true, force: true });
});

it('yes (--dry-run)', async () => {
const context = { cwd: '', dryRun: true, prompt: () => ({ git: true }) };
await git(context);
Expand All @@ -46,3 +29,29 @@ describe('git', () => {
expect(fixture.hasMessage('Skipping Git initialization')).to.be.true;
});
});

describe('git initialized', () => {
const fixture = setup();
const dir = new URL(new URL('./fixtures/not-empty/.git', import.meta.url));

before(async () => {
await mkdir(dir, { recursive: true });
await writeFile(new URL('./git.json', dir), '{}', { encoding: 'utf8' });
})

it('already initialized', async () => {
const context = {
git: true,
cwd: './test/fixtures/not-empty',
dryRun: false,
prompt: () => ({ git: false }),
};
await git(context);

expect(fixture.hasMessage('Git has already been initialized')).to.be.true;
});

after(() => {
rmSync(dir, { recursive: true, force: true });
})
})
Comment on lines +33 to +57
Copy link
Member Author

Choose a reason for hiding this comment

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

Updated this test just for clarity

Loading
Loading