Implement Feature Request: Add custom fields to define index and error documents

[evanseeds]

Added three .yml options: 'indexDocument' and 'errorDocument', which allow customizing the s3 bucket option for each, and 'spa', which automatically sets the errorDocument to be the indexDocument, overriding the errorDocument .yml setting if both are set.

This implements the issue at #16

[fernando-mc]

@evanseeds only one substantial question. After you answer that we can make changes here and get this into the next release.

[fernando-mc]

@evanseeds Is there a benefit to adding this extra configuration value over just letting the user set both the index and error document to index.html?

It seems like it's an extra config value? Here's what I mean:

This:

indexDocument: index.html
spa: true

Is the same expected outcome as this:

indexDocument: index.html
errorDocument: index.html

Any objection to removing the spa option? I'm open to it with valid use-case. I might just not understand it presently.

[evanseeds]

My use case is using finch to deploy single-page applications that need all traffic redirected to index.html. This allows the webapp in the index.html file to handle all routes. I think it's more intuitive to be able to just set a 'spa' flag to true. Otherwise, a user would be to have to figure out how to implement this feature in S3, and then how to translate those S3 settings into finch.

In most of these cases you would be using the default 'index.html' indexDocument, and so the config comparison would be:

spa: true

vs

indexDocument: index.html
errorDocument: index.html

The specific changes being made are more obvious in the second, but I think having the first option is more user-friendly.

'spa' may not be the best name, for sure. 'singlePage' or 'singlePageApplication' would be clearer names. Alternative, a more descriptive name like 'redirectErrorsToIndex' may work too.

Ultimately, there's a part of me that just doesn't like having to write the same thing twice, but maybe that's just an obsession 😅

[fernando-mc]

If spa: true the errorDocument will be ignored.

See my above comment on related to the spa config value.

[fernando-mc]

Thanks for saving me the work 😉

[evanseeds]

I try to be thorough 😅

[stefan2718]

Once this PR is merged, approximately how soon after would it be in a release? I'm eagerly awaiting this feature.

[fernando-mc]

I think the SPA option may be a bit confusing and more difficult to write tests for in the future (which I'm trying to move towards as development on this increases)

For now, I'm going to remove that option but I'm open to revisiting the decision in the future. The two options this adds (without the spa option) should be enough to get the same functionality.

[fernando-mc]

@evanseeds can you test this again? I'm seeing an error. Testing using a simple config:

service: serverless-finch-test

provider:
  name: aws
  runtime: nodejs6.10

plugins:
  - serverless-finch

custom:
  client:
    bucketName: my-serverless-finch-test-eu-west-3-test
    distributionFolder: website
    indexDocument: index.html
    errorDocument: error.html

./website contains the index.html and error.html files. I would expect this to work but somehow the indexdoc variable is undefined at that time.

ReferenceError: indexdoc is not defined
    at Client.configureBucket (/home/fernando/Documents/code/test-serverless-finch/node_modules/serverless-finch/index.js:193:18)

@stefan2718 once I'm sure this doesn't have any bugs I'll ship it. You're welcome to help write tests to help catch things like this! Unfortunately, manual tests take a while.

[evanseeds]

I updated it to remove the spa option and fixed that bug as well. It should be good now!

[fernando-mc]

@evanseeds tested and merged. Checking a few things and pushing a new version to npm.

[akinnee]

How about an option to specify the HTTP code when returning the errorDocument? Since I'm hosting a SPA on s3, I want it to be 200.

[fernando-mc]

Thanks for the question @akinnee. You're welcome to open a new issue for this or a PR that implements the functionality. I'm not entirely sure how that would work as I think S3 handles the codes returned from the site and I don't know of a way to customize those codes. But if you're aware of a way to do this please open an issue and we can look into it!