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

fix(gatsby-link): Correct handling of trailingSlash & pathPrefix #36542

Merged

Conversation

andrei-piatrou
Copy link
Contributor

@andrei-piatrou andrei-piatrou commented Sep 5, 2022

Description

When trailingSlash: never and PREFIX_PATH is defined root page address is not properly generated. Instead of /prefix it gets geenrated as /prefix/

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Sep 5, 2022
@andrei-piatrou andrei-piatrou force-pushed the fix/withprefix-fix-for-root-page branch 2 times, most recently from 8afe571 to 0c736be Compare September 5, 2022 15:30
@LekoArts LekoArts added topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Sep 6, 2022
@LekoArts
Copy link
Contributor

LekoArts commented Sep 6, 2022

Thanks for the PR! Looking good so far.

Can you also add test cases to the other options (legacy, always, ignore)? And about the current test failures: These are unrelated to your PR, we're currently fighting with some upstream dependencies that changed.

@LekoArts LekoArts self-assigned this Sep 6, 2022
@LekoArts LekoArts changed the title fix(gatsby-link) when prefix_path+trailing_slash: never applied fix(gatsby-link): Correct handling of trailingSlash & pathPrefix Sep 6, 2022

Unverified

This user has not yet uploaded their public signing key.
@andrei-piatrou andrei-piatrou force-pushed the fix/withprefix-fix-for-root-page branch from 8fbf6ef to bba6601 Compare September 7, 2022 07:25
@andrei-piatrou
Copy link
Contributor Author

andrei-piatrou commented Sep 7, 2022

Thanks for the PR! Looking good so far.

Can you also add test cases to the other options (legacy, always, ignore)? And about the current test failures: These are unrelated to your PR, we're currently fighting with some upstream dependencies that changed.

I am not sure that failing test is related to my changes. Could it be that it has come from the master branch?

@LekoArts
Copy link
Contributor

LekoArts commented Sep 7, 2022

e2e_tests_development_runtime_with_react_18 and ci/circleci: e2e_tests_production_runtime_with_react_18 have some flaky tests, so should be all good :)

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.

Should be all good. Thanks 👍

@LekoArts LekoArts merged commit 85b1319 into gatsbyjs:master Sep 8, 2022
@LekoArts
Copy link
Contributor

LekoArts commented Sep 8, 2022

You can try it out with gatsby@next

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants