-
-
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
Conversation
🦋 Changeset detectedLatest commit: fe5ae23 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
// This is an extremely simplified version of [`execa`](https://github.com/sindresorhus/execa) | ||
// intended to keep our dependency size down |
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.
Node built-ins are pretty powerful these days, so execa
isn't really needed
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.
Doing this for Astro itself would probably be also great
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.
Agreed! We can follow-up on that
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 }); | ||
}) | ||
}) |
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.
Updated this test just for clarity
"chai": "^4.3.7", | ||
"mocha": "^9.2.2", |
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.
These should have always been dev deps!
"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 comment
The reason will be displayed to describe this comment to others. Learn more.
giget
was pinned, but that's no longer needed
@@ -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", |
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!
Changes
mocha
andchai
todevDependencies
giget
sincedownloadTemplate
broken since last update unjs/giget#81 was merged in February, but we never updatedexeca
to a thinchild_process
wrapper since we don't need more advanced featuresTesting
Existing tests pass
Docs
N/A, maintenance only