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(gatsby-core-utils,gatsby-cli): Allow write to gatsby-config.ts #35074

Merged
merged 16 commits into from
Mar 14, 2022

Conversation

tyhopp
Copy link
Contributor

@tyhopp tyhopp commented Mar 8, 2022

Description

Allow customers using create-gatsby to have their preferences selected during the questionnaire written programatically to the (new) gatsby-config.ts file in gatsby-starter-minimal-ts.

TS Gatsby config extracted out to #35128.

Documentation

No updates required, relevant doc is https://www.gatsbyjs.com/docs/how-to/custom-configuration/typescript/#gatsby-configts

Related Issues

[sc-47001]

Tests

As far as I can tell there aren't any tests for existing functionality, so I'll add those and also cover this new case.

Will probably import the starter configs in tests so that if we change the starter configs in the future, the test breaks and we make sure we update the transform if need be.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Mar 8, 2022
@tyhopp tyhopp marked this pull request as draft March 8, 2022 08:50
path.stop()
},
},
function addPluginsToConfig({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is extracted out from existing transform so it can be used for gatsby-config.js and gatsby-config.ts, nothing should be changed

@tyhopp tyhopp added topic: cli Related to the Gatsby CLI topic: TypeScript Issues and PRs related to TS in general, public typings or gatsby-plugin-typescript type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement. and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Mar 8, 2022
@tyhopp tyhopp changed the title feat(gatsby-core-utils,gatsby-cli,create-gatsby): Allow write to gatsby-config.ts feat(gatsby-core-utils,gatsby-cli): Allow write to gatsby-config.ts Mar 9, 2022
@tyhopp tyhopp marked this pull request as ready for review March 9, 2022 06:52
Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

Didn't get to a full review yet, wanted to mention two things already

@@ -0,0 +1,104 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For anyone reading this (or my future self), I took a look at making the quote type of the output strings in the transform consistent (e.g. all backticks).

The only obstacle is in the buildPluginNode function we JSON.stringify the options object, and if we try to make object keys backticks then the transform fails since that's invalid JSON (and we can't expect keys to only be alphabetic).

Anyhow not a huge deal, but wanted to leave this for posterity. Will leave this out of this PR.

Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

It just hit me: We can’t merge yet with the changed ts starter as otherwise until this is released the workflow is broken

@tyhopp
Copy link
Contributor Author

tyhopp commented Mar 14, 2022

It just hit me: We can’t merge yet with the changed ts starter as otherwise until this is released the workflow is broken

That's true, good catch. I suppose we could separate out just the TS starter change, we'd have to remove or comment out the new test for that as well. Or keep it all together here and merge this after the release. Either way we'd have to merge at least one PR after the release 🤔

@tyhopp tyhopp added the status: blocked This issue/PR can't be solved at the moment and shouldn't be closed/merged label Mar 14, 2022
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 topic: TypeScript Issues and PRs related to TS in general, public typings or gatsby-plugin-typescript type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants