gatsby-uc / gatsby-plugin-s3

Deploy your gatsby site to a S3 bucket.
https://gatsby-plugin-s3.jari.io/
MIT License
210 stars 109 forks source link

Client-only routes cause 302 redirects or 404 responses #77

Open karldanninger opened 5 years ago

karldanninger commented 5 years ago

I'm building an app on top of Gatsby using this package to bundle and deploy the front-end.

Reproduction:

  1. Clone this starter by Nader Dabit that contains some client-side routing https://github.com/dabit3/gatsby-auth-starter-aws-amplify
  2. Deploy it with gatsby-plugin-s3

Current behavior:

RoutingRules will be generated on the s3 bucket which will cause 302 redirects on /app/login to /app. Same thing if you try to hit /app/login directly.

Potential fix:

Add a flag to not generate RoutingRules via the CLI?

Manual fix:

After every deployment, remove certain routes on the s3 buckets RoutingRules

Closing Remarks:

Thank you so much for your hard work. I love/recommend this package a lot.

jariz commented 5 years ago

This is by design. The package you mentioned makes use of a gatsby 'feature' called client only routes.

This is probably behaviour you want, because without it if people navigate from /app to /app/login and then hit refresh, they will get a 404. (because anything after /app does not exist on the server side)

If it's not what you want, there's always the option to disable this behaviour:

https://github.com/jariz/gatsby-plugin-s3/blob/53c08f55486b698c7e5957f04202fe81a7b98796/src/constants.ts#L62

(or you can just remove the createPage call from your gatsby-node)

karldanninger commented 5 years ago

OK, thank you @jariz. I appreciate it <3

I basically want a gatsby page that takes an id, /stuff/1234 and to render that page with content based on that id.

This makes sense now

YoshiWalsh commented 5 years ago

Hey @jariz, I'm thinking about this one. In an ideal world we would preserve the path. In httpd you would do this using a rewrite, so instead of returning a 301/302 it would perform the redirect within the web server and just return the contents of the destination page.

S3 doesn't support this and I was wondering how we could. Initially it seemed to me it would need a Lambda@Edge Origin Request function for this, but I've thought of a simpler way:

What if we just copy the contents of the rewrite destination to the original path? That should function the same as a traditional rewrite. Obviously the new behaviour should be optional, but I think it would help for many clientside-only route use cases.

oorestisime commented 4 years ago

Hey so i am trying to move away from amplify to use s3/cloudfront and i am searching to do this for a site. enlighten me if i am wrong but it seems to me right now that client only routes are a no go for s3/cloudfront with current configuration and this plugin. On amplify you can have a url rewrite that you can use to set up client/* to client/index.html and problem solved. Now on s3/cloudfront when i arrive directly to client/something i get a 403 from s3 and then a 404 from cloudfront (custom responses). So i am wondering how this could be done while having some doubts about using Lambda@Edge.

Amplify seems to be managing s3/cloudfront under the hood so i am probably missing something.

any help is appreciated.

karldanninger commented 4 years ago

Hey guys, I'm wondering more about this /user/123 thing. I just hit an issue with a user trying to add the link into a Word Doc, and Word is reporting the link as a 404.

If you curl -I https://www.atlistmaps.com/map/fd52357d-d3e1-4c72-940a-4c0f196bbdab it also reports a 404, even though the same link renders in the browser. Is there a way to config gatsby-plugin-s3 to report 200, 301, or 302 for certain client-side paths?

Another insight: gatsby build && gatsby serve, a curl -I reports 200.

My current config looks like this:

    {
      resolve: 'gatsby-plugin-create-client-paths',
      options: { prefixes: ['/map/*'] },
    },
    {
      resolve: 'gatsby-plugin-s3',
      options: {
        bucketName: process.env.S3_BUCKET_NAME || 'dev',
        protocol: siteAddress.protocol.slice(0, -1),
        hostname: siteAddress.hostname,
        generateMatchPathRewrites: false,
      },
    },

@jariz & @JoshuaWalsh how can I tip you? I wholly appreciate your work on this package.

Edit:

Setting generateMatchPathRewrites: true will redirect to /map on a hard refresh, which is not ideal.

YoshiWalsh commented 4 years ago

Hi @karldanninger ,

I think we'd have to implement my suggestion in order to report 200 without redirecting.

I don't have the time to implement this at the moment, but I'd be happy to review a PR that implements this.

@oorestisime I haven't used Amplify, are you able to look at the result and see what it's doing? Is it using Lambda@Edge, a duplicated object, or something else that we don't know about?

pierluigigiancola commented 4 years ago

Hi everyone, I create an example project to reproduce #133 . I've tested it and it loops because of the rules written on the bucket.

We generate the redirect rules here. This is the only place we generate them. Since we just generate rules for whatever Gatsby gives us, looks like this isn't a bug in our plugin.

Even if it's not a bug in your plugin, I'll leave the project here so that anyone intrested can investigate.

If I'll find out anything I'm gonna write the info on this issue.

Thank @JoshuaWalsh for your help, have a nice day everyone.

ianpogi5 commented 4 years ago

Hi guys I posted on SO my solution.

https://stackoverflow.com/a/61694958/130237

...but still have an issue which may not be related to this plugin.

sladg commented 3 years ago

I'm experiencing the same issue. gatsby-plugin-create-client-paths with /app/** prefix. I get https://domain/app which returns 302, because of subfolders index.html. After trying to get https://domain/app/ to get index.html, I get redirected to the firstly visited route.

sladg commented 3 years ago

Is there any progress on this issue? :)