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

Add config variable to avoid canonical redirect #15173

Closed
wants to merge 2 commits into from
Closed

Add config variable to avoid canonical redirect #15173

wants to merge 2 commits into from

Conversation

xavivars
Copy link
Contributor

Description

This PR introduces a config variable that allows you to disable redirect when a page loads, in case the paths are different when generated and when displayed.

This used together with https://github.com/wardpeet/gatsby-plugin-static-site allows to use gatsby as a static page generator instead of a static site generator

Related Issues

This PR is basically the same one as #11914 and fixes #4337

@xavivars xavivars requested a review from a team as a code owner June 27, 2019 00:11
@KyleAMathews
Copy link
Contributor

Generally speaking we don't add config options. Is there another way we can solve this?

@xavivars
Copy link
Contributor Author

I'd love to have another way to do this, but I can't think of another way to break this if

!__DISABLE_LOAD_TIME_CANONICAL_CHECK_REDIRECT__ &&

Up until 2.9, there was a workaround, that involved changing what is currently window.pagePath to browser's location hooking into clientEntry but that stopped working due to the new per-page page-data.json relying on that window.pagePath.

I don't now if creating a new hook at that point in the code is preferable to a config variable...

@xavivars
Copy link
Contributor Author

Maybe being able to override here the value of browserLoc to not to use the real browser window.location could also work, but I have no idea on how that could be done...

const { pagePath, location: browserLoc } = window

@xavivars
Copy link
Contributor Author

xavivars commented Jun 27, 2019

@KyleAMathews, this PR takes a much simpler approach, and enables the same use case

#15180

Do you think the approach is better?

@wardpeet
Copy link
Contributor

wardpeet commented Jul 3, 2019

Hey @xavivar, thank you for opening this pull request!

Unfortunately, the proposed change is not going to be merged. This is a fairly unusual usecase, and while we'd love to support every usecase for everyone we don't have the capacity to do that right now. It may seem like a small change but any time we expose apis publicly like this it means we're committed to supporting them, which can lead to problems when we change them later on.

Thanks again, and we look forward to seeing more PRs from you in the future! 💪💜

@wardpeet wardpeet closed this Jul 3, 2019
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.

Disable Client-Side Routing?
3 participants