-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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", | ||
"giget": "1.0.0", | ||
"mocha": "^9.2.2", | ||
"giget": "^1.1.2", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
"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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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", | ||
|
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Node built-ins are pretty powerful these days, so There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doing this for Astro itself would probably be also great There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{} |
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'; | ||
|
@@ -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); | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated this test just for clarity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this!