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(redirects): handle absolute from paths when path prefix is used #12509

Merged
merged 3 commits into from
Mar 13, 2019

Conversation

edykim
Copy link
Contributor

@edykim edykim commented Mar 12, 2019

Description

toPath in createRedirect handles path prefix properly, however, fromPath doesn't. I added the code for supporting the fromPath cases.

Also, I added test cases of createRedirect action for the future.

Related Issues

Fixes #12497

@edykim edykim requested a review from a team as a code owner March 12, 2019 07:31
Copy link
Contributor

@pieh pieh 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 looking very nice! I really love comprehensive test suite for this! I have one suggestion just to keep code a little more DRY

packages/gatsby/src/redux/actions.js Outdated Show resolved Hide resolved
packages/gatsby/src/redux/__tests__/redirects.js Outdated Show resolved Hide resolved
Remove duplicated logic and make it as maybeAddPathPrefix.
@edykim
Copy link
Contributor Author

edykim commented Mar 12, 2019

@pieh Thanks for the thoughtful review. I updated the code based on your recommendation. Please review the changes when you have time. 😊

@edykim edykim requested a review from pieh March 13, 2019 06:52
@pieh pieh changed the title Handle path prefix on fromPath in createRedirect feat(redirects): handle absolute from paths when path prefix is used Mar 13, 2019
Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

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

Awesome! thanks @edykim!

@pieh pieh merged commit c6583d4 into gatsbyjs:master Mar 13, 2019
@edykim edykim deleted the topics/gatsby-actions-create-redirect branch March 13, 2019 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

createRedirect from other domain conflicts with pathPrefix
2 participants