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

feat(create-gatsby): Add create-gatsby #27703

Merged
merged 102 commits into from Nov 12, 2020
Merged

feat(create-gatsby): Add create-gatsby #27703

merged 102 commits into from Nov 12, 2020

Conversation

ascorbic
Copy link
Contributor

@ascorbic ascorbic commented Oct 28, 2020

This is a new command create-gatsby, which creates and configures a new site, a little like gatsby new on dexamethasone.

To run it:

npm init gatsby

or

yarn create gatsby

Goals

The command needs to run quickly, and get the user with a working site, configured to their needs, with a minumum number of steps.

The command aims to be as small as possible, because it has to download and install it before running. The target is <100kB, gzipped including dependencies. The current size is 75kB 95kB.

There should be no need for flags or arguments, because these are not easily discoverable. Instead it should use interactive configuration.

Architecture

To keep the install fast, the create-gatsby package does not depend on any gatsby packages except core-utils. When running, it uses enquirer to ask a number of questions to determine plugins to install. It starts by cloning and installing gatsby-starter-hello-world. It uses a stripped-down version of the initStarter command from gatsby-cli for this.

To install plugins it will use the copy of gatsby-cli in the newly-installed site. To do this, it resolves the gatsby-cli package relative to the site root, then loads create-cli. It then passes the plugin add args to that function, which runs it as if it had been invoked directly.

[ch17789]
[ch17925]
[ch17779]

@ascorbic ascorbic requested a review from a team as a code owner October 28, 2020 17:34
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 28, 2020
@ascorbic ascorbic marked this pull request as draft October 28, 2020 17:34
packages/create-gatsby/src/index.ts Outdated Show resolved Hide resolved
packages/create-gatsby/src/index.ts Outdated Show resolved Hide resolved
packages/create-gatsby/src/questions.json Outdated Show resolved Hide resolved
Copy link
Contributor

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

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

This looks fantastic already! 😍

packages/create-gatsby/src/questions.json Outdated Show resolved Hide resolved
packages/create-gatsby/src/questions.json Outdated Show resolved Hide resolved
Copy link
Contributor

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

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

It would also be fantastic if we could already figure out how to automatically test this to ensure it works as expected. How we're going to best do that is honestly a bit of a questionmark in my head, I've never unit tested a CLI...

@ascorbic
Copy link
Contributor Author

If we can work out how to do the interactive input we could test it by calling the run command, and mock execa so we can see what git commands it's trying to run.

@ascorbic
Copy link
Contributor Author

Nice! https://medium.com/@zorrodg/integration-tests-on-node-js-cli-part-2-testing-interaction-user-input-6f345d4b713a

@ascorbic ascorbic added topic: cli Related to the Gatsby CLI and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 29, 2020
@ascorbic
Copy link
Contributor Author

The mock-stdin package did the trick.

@@ -0,0 +1,8 @@
{
"none": "No (or I'll add it later)",
Copy link
Contributor

Choose a reason for hiding this comment

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

enquirer seems to have some internal logic to use the message if the name is falsy, so I just added a string to check against.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha. I was really confused this morning. I'd done literally the same fix yesterday but forgot to commit it. When I pulled it had already been changed, and I couldn't work out why it was still showing as uncommitted locally! Good solution 😉

@ascorbic ascorbic changed the title WIP: Add create-gatsby feat(create-gatsby): Add create-gatsby Nov 10, 2020
@ascorbic
Copy link
Contributor Author

I've disabled the tests until we can work out how to reproduce the failures

mxstbr
mxstbr previously approved these changes Nov 12, 2020
Copy link
Contributor

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

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

Small nits only, let's merge this into master?!

packages/create-gatsby/README.md Outdated Show resolved Hide resolved
packages/create-gatsby/scripts/import-options-schema.js Outdated Show resolved Hide resolved
* feat(create-gatsby): Add support for plugin dependencies

* Fix extra plugin handling
@laurieontech laurieontech merged commit 2371fd5 into master Nov 12, 2020
@delete-merged-branch delete-merged-branch bot deleted the feat/create-gatsby branch November 12, 2020 17:56
@laurieontech laurieontech restored the feat/create-gatsby branch November 12, 2020 17:57
installTimer.start()
reporter.info(`Installing ${plugin}`)
try {
const result = await NPMPackage.create({ root }, { name: plugin })
Copy link
Contributor

Choose a reason for hiding this comment

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

@ascorbic a bit late to the game but is there a reason this create call is awaited? It makes package installs serial — if you just return the promise, the NPMPackage provider will batch up all calls made at the same time into one npm/yarn install call.

Copy link
Contributor Author

@ascorbic ascorbic Nov 25, 2020

Choose a reason for hiding this comment

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

@KyleAMathews Afaict, this should still work, because we're doing the calls to installPluginPackage in parallel in an async function. It will "await", but the async function will have already returned the promise so they'll still run in parallel

Copy link
Contributor

Choose a reason for hiding this comment

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

ahhh yeah 😅 async is hard. Carry on!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When it messes with my head, I find it helpful to think of an async function as starting with return new Promise...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: cli Related to the Gatsby CLI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants