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 wip plugin configuration forms #27801

Merged

Conversation

gillkyle
Copy link
Contributor

@gillkyle gillkyle commented Nov 3, 2020

Description

This is a mess of poorly typed code that introduces a WIP form of prompting users for required plugin options when they select plugins in the create gatsby flow.

@gillkyle gillkyle requested a review from a team as a code owner November 3, 2020 00:31
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Nov 3, 2020
@LekoArts LekoArts 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 Nov 3, 2020
Copy link
Contributor

@ascorbic ascorbic left a comment

Choose a reason for hiding this comment

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

This is great! Pretty much perfect, imo. Just added a few stylistic comments. I've made a few changes to the typings. Behold the magic of keyof typeof, that lets you use the inferred keys from the JSON file! I've also added the Joi typings for the schema.

): Array<any> => {
const formPrompts = selectedPlugins.map((pluginName: string) => {
const schema = pluginSchemas[pluginName]
const { keys: options } = schema
Copy link
Contributor

Choose a reason for hiding this comment

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

The plugins aren't all objects, and don't all have a keys prop, so we should check for this at runtime. This will also narrow the type for TypeScript.

Suggested change
const { keys: options } = schema
if (typeof schema === `string` || !(`keys` in schema)) {
return []
}
const { keys: options } = schema

const formPrompts = selectedPlugins.map((pluginName: string) => {
const schema = pluginSchemas[pluginName]
const { keys: options } = schema
const choices = Object.keys(schema?.keys).reduce((result, name) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

In a simple case like this where you're just pushing things onto an array, a forEach is easier to read

const choices = Object.keys(schema?.keys).reduce((result, name) => {
const option = options[name]

if (option?.flags?.presence === `required`) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Another readability suggestion: you could reverse the check here and do an early return, instead of wrapping the whole lot in the conditional

@@ -71,6 +108,18 @@ export async function run(): Promise<void> {
console.log(`Let's answer some questions:\n`)
const data = await prompt<IAnswers>(questions)

const selectedPlugins = [data.cms, data.styling, ...data.features].filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm already creating an array of selected plugins below, so you could use that instead

@ascorbic ascorbic merged commit 99de6b7 into feat/create-gatsby Nov 3, 2020
@delete-merged-branch delete-merged-branch bot deleted the feat/create-gatsby-plugin-configurations branch November 3, 2020 10:42
laurieontech pushed a commit that referenced this pull request Nov 12, 2020
* Add create-gatsby

* add question and readme, update some wording

* Update readme

* Update handling of questions

* Stylish!

* Update message

* Update messages

* Catch ctrl^c

* v0.0.0-2

* Add test

* Fix test

* feat(gatsby): Add "gatsby plugin add" command

* Load readme from local package

* ensure skipped steps are actually skipped

* attempt to add some more tests

* Move command to cli

* Try to install plugins

* feat(create-gatsby): add wip plugin configuration forms (#27801)

* add wip plugin configuration forms

* Use plugins array

* Typings

Co-authored-by: Matt Kane <matt@gatsbyjs.com>

* Install plugins

* Add error handling

* Return, don't exit

* Fix tests

* Resolve themes relative to root

* Change back to original dir

* Use starter with canary

* v0.0.0-3

* Fix to force publish

* Working!

* Change from review

* Fix package name

* Use gatsby-source-wordpress-experimental

* Add schema import script

* v0.0.0-4

* Add dep

* v0.0.0-5

* handle peer dependencies

* forgot to save a file

* bump core-utils dependency

* update styling and text to match Flo's design, in progress

* use magenta for all actions to be taken

* add final prompts

* consistent coloring

* consistent coloring

* consistent coloring

* Initial input test

* Layout helpers

* Add custom textinput prompt

* Fixes to hint

* Fix error

* Add select control

* update options for gatsby new

* Tab to end

* update tests

* send both tabs

* it was the right hex code, or not

* order shouldn't matter but I'm very confused

* will slash tab work

* Formatting fixes

* v0.0.0-6

* use down inside of tab

* test enter

* trying one more thing, but suspect it's unrelated

* Update packages/create-gatsby/src/cmses.json

Co-authored-by: Lennart <lekoarts@gmail.com>

* Update packages/create-gatsby/src/styles.json

Co-authored-by: Lennart <lekoarts@gmail.com>

* one more time

* bane of my existence

* try different keys for different OS

* somehow bypasses linter

* remove console.log

* add fake plugin schemas for other cmses

* Fix test

* Longer tick

* Add description to form inputs

* Test changes

* increase interval

* add initial working test

* missing return type

* not sure why linter keeps skipping these diles

* all tests except project name exists

* just in case it's a timeout thing

* does a single question pass

* have CI run individual questions and see if we falter

* Test

* Burn it with fire 🔥

* Move components into plugin

* Replace Prismic with Dato

* Install plugins at the start

* Add support for plugin dependencies (#27995)

* feat(create-gatsby): Add support for plugin dependencies

* Fix extra plugin handling

* Apply suggestions from code review

Co-authored-by: Max Stoiber <contact@mxstbr.com>

* Remove broken tests 😢

Co-authored-by: Kyle Gill <kylerobertgill@gmail.com>
Co-authored-by: Laurie <laurie@gatsbyjs.com>
Co-authored-by: gatsbybot <mathews.kyle+gatsbybot@gmail.com>
Co-authored-by: Lennart <lekoarts@gmail.com>
Co-authored-by: Max Stoiber <contact@mxstbr.com>
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

3 participants