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

Feature/prevent navigation #11914

Closed

Conversation

ethagnawl
Copy link

@ethagnawl ethagnawl commented Feb 19, 2019

Description

This new top-level configuration option will allow Gatsby to (more easily) be used as a static page generator -- as opposed to a static site generator. The __DISABLE_ALL_REDIRECTS_BECASUE_IM_USING_GATSBY_AS_A_STATIC_PAGE_GENERATOR__ (config name is WIP and I'm very open to suggestions) will prevent Gatsby from trying to make the canonical path match the actual path.

To quickly summarize a use case which would necessitate this functionality: I want to (did!) use Gatsby as a static page generator and because the Gatsby "page" is injected into a containing page whose URL is out of my control and subject to change, I don't want the Gatsby application ever mucking with the URL. Out of the box, Gatsby mostly supports this use case and is a great pleasure to use, but it does makes some assumptions -- again, because of its standard static site use case -- that result in the need for hacks involving window.page.path.

I used #10933 as a reference for where top-level config options would need to be introduced, validated, etc. but it's possible that I'm not doing this correctly or in the correct places. (e.g. should I be checking for the truthiness of the new config flag via the program object in packages/gatsby/src/utils/webpack.config.js?)

I'd also appreciate pointers on how this new behavior can be tested.

Related Issues

Fixes #4337

…avigation when gatsby is being used as a single/static page generator
…e working example application using new config option
@ethagnawl ethagnawl requested a review from a team as a code owner February 19, 2019 22:34
@pieh
Copy link
Contributor

pieh commented Feb 19, 2019

@ethagnawl can you at least temporarily remove example site from changes? It makes it pretty hard confusing to see where actual / meaningful changes are when 90% of changes is example code

@ethagnawl
Copy link
Author

ethagnawl commented Feb 20, 2019

@pieh I've removed some of the extraneous files from the example, which should help with readability.

I'd prefer to include the example since there aren't currently any tests covering this behavior and it's a simple way for reviewers to verify that the feature actually does what it claims to.

@pieh
Copy link
Contributor

pieh commented Feb 20, 2019

I'd prefer to include the example since there aren't currently any tests covering this behavior and it's a simple way for reviewers to verify that the feature actually does what it claims to.

It should be matter of adding toggle in config (in current shape), so just documenting to adjust gatsby-config will be enough to be able to test it.

There are major problems with this - it won't work for any resources that are referenced (images, fonts, js bundles) because we use absolute paths for those. I'm not sure if it's possible to actually fix those

If you have image referenced in component X, and use that component in /index.html and /nested/index.html` - how could this work - the path to resource would need to be different in both pages.

@ethagnawl
Copy link
Author

There are major problems with this - it won't work for any resources that are referenced (images, fonts, js bundles) because we use absolute paths for those. I'm not sure if it's possible to actually fix those

If you have image referenced in component X, and use that component in /index.html and /nested/index.html` - how could this work - the path to resource would need to be different in both pages.

All this feature is doing is optionally preventing the client JS from calling reach/router::navigate and changing the page's URL. I do not think this will affect asset references.

If there was an issue with asset references (e.g. the Gatsby application lived at a dynamic location) I'd think the user would be able to sort that out using the existing pathPrefix option and/or the soon to be available (:crossed_fingers:) assetPrefix option.

For whatever it's worth, I've used a similar approach in a few simple applications -- two of which used @DSchau's proposed assetPath feature -- and didn't have any issues with asset references (CSS, fonts or images).

@pieh
Copy link
Contributor

pieh commented Feb 21, 2019

Hmm, I probably don't understand use case for it then. You still need to run some kind of http server to run it - if that's the case why this is needed?

Because of my lack of understanding this might completely miss the mark - but you can make your canonical path for a page to be /index.html (instead of /) right now.

@ethagnawl
Copy link
Author

To greatly simplify the various use cases outlined in the linked issue: people want to use Gatsby to generate static pages (e.g. a marketing landing page, a one-off page within a legacy CMS, a fragment of a page, etc.). These pages often live at indeterminate/dynamic locations, so it doesn't make sense and is not desirable for Gatsby to manage/manipulate the page's URL.

You could argue that the above is outside of Gatsby's purview, but lots of people want to use/are using Gatsby in this way and it'd be nice to offer them a uniform way to achieve this -- as opposed to relying on brittle hacks like the ones offered in the linked issue.

@m-allanson
Copy link
Contributor

Thanks @ethagnawl - I love the name of the config key :D

I'm not convinced about adding this feature to Gatsby core, however I think this is a great case for creating a plugin. Adding a feature to Gatsby core means we're committed to testing, documenting and maintaining that feature for a long time. As this isn't (currently) a major use-case for Gatsby, the plugin approach seems like a good balance of discoverability and ease-of-use, while allowing the community to handle the maintenance and support side.

A nice side-effect of doing this as a plugin is that we can use npm download stats to get an idea of how popular it is. If it turns out that suddenly everyone is doing this with their Gatsby sites, then that's an excellent reason to merge the plugin into Gatsby as a built-in feature.

@wardpeet has some ideas on how to implement this as a plugin and is planning to create a proof-of-concept.

@ethagnawl
Copy link
Author

@m-allanson That sounds like a reasonable compromise to me.

@sidharthachatterjee
Copy link
Contributor

Looks like we're all on the same page then 🙂

Closing this for the time being but thank you once again for your work on this @ethagnawl and we'll look forward to other contributions from you!

@xavivars
Copy link
Contributor

@ethagnawl : I think the workaround needed on top of the plugin when URLs in gatsby do not match URLs in where the pages are exposed doesn't work anymore. #15180 enables that again. Could you check if it works for you?

@ethagnawl
Copy link
Author

@xavivars I'll try to find some time to pull your branch down and experiment. I haven't used Gatsby in almost six months, so what is the quickest way to validate the proposed changes? (Also, why does the related example have a dependency on gatsby-plugin-s3?)

@xavivars
Copy link
Contributor

I added s3 because I pushed it to s3, and then added a proxy in front of it. But actually you don't need that. Just accessing index.html it shouldn't redirect (while Gatsby does it by default)

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?
5 participants