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): Supports multiple development proxies #20369

Merged
merged 10 commits into from
Jan 10, 2020

Conversation

ThewBear
Copy link
Contributor

@ThewBear ThewBear commented Jan 1, 2020

Description

Makes development proxy config accepts either an object or array of objects.

Documentation

https://www.gatsbyjs.org/docs/api-proxy/

Related Issues

#20366

@ThewBear ThewBear requested a review from a team January 1, 2020 10:40
@ThewBear ThewBear requested a review from a team as a code owner January 1, 2020 10:40
)
.on(`error`, (err, _, response) => {
if (response) {
proxy.forEach(({prefix, url}) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if only an object is set in the config?
Would still behave as an array?

Copy link
Contributor Author

@ThewBear ThewBear Jan 2, 2020

Choose a reason for hiding this comment

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

Object would be converted to array by joi array.single()

url: Joi.string().required(),
})
)
.single(),
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be my dev preference. Having a more specific type either array or object gives more predictability to the code execution. I would rather release it as a breaking change.

Having only:

proxy: Joi.array()
      .items(
        Joi.object().keys({
          prefix: Joi.string().required(),
          url: Joi.string().required(),
        })
      ),

daniloster
daniloster previously approved these changes Jan 3, 2020
@daniloster
Copy link
Contributor

Sorry taking long on this

@ThewBear ThewBear added status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response type: feature or enhancement labels Jan 7, 2020
freiksenet
freiksenet previously approved these changes Jan 8, 2020
Copy link
Contributor

@freiksenet freiksenet left a comment

Choose a reason for hiding this comment

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

Thank you!

There are some merge conflicts from additional fixes to proxying, this will be fine to merge once those are done.

@ThewBear ThewBear dismissed stale reviews from freiksenet and daniloster via e13a313 January 8, 2020 15:15
@ThewBear ThewBear requested a review from freiksenet January 10, 2020 02:00
@freiksenet freiksenet added bot: merge on green Gatsbot will merge these PRs automatically when all tests passes and removed status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response labels Jan 10, 2020
@gatsbybot gatsbybot merged commit 8d1e37a into gatsbyjs:master Jan 10, 2020
@ThewBear ThewBear deleted the ThewBear-20366 branch January 10, 2020 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants