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.

Sign up for GitHub

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

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

Merged
merged 4 commits into from
Feb 13, 2018
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
162a3e9
Add support for customizing index and error documents and included an…
evanseeds Jan 31, 2018
bef8d7c
Clarify wording in readme regarding spa option
evanseeds Jan 31, 2018
2e0ae1e
Remove spa option
evanseeds Feb 13, 2018
5dc7089
Merge branch 'master' into master
fernando-mc Feb 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,15 @@ custom:
client:
bucketName: unique-s3-bucketname-for-your-website-files
distributionFolder: client/dist # (Optional) The location of your website. This defaults to client/dist
indexDocument: index.html # (Optional) The location of your index document. Defaults to index.html
errorDocument: error.html # (Optional) The location of your error document. Defaults to error.html
spa: false # (Optional) Set this for single-page applications to redirect all errors to the index document
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 😅

```

* **Warning:** The plugin will overwrite any data you have in the bucket name you set above if it already exists.

* **Warning:** If the 'spa' option is set, the errorDocument setting will be ignored.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If spa: true the errorDocument will be ignored.

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



**Third**, Create a website folder in the root directory of your Serverless project. This is where your distribution-ready website should live. By default the plugin expects the files to live in a folder called `client/dist`. But this is configurable with the `distributionFolder` option (see the example yaml configuration above).

Expand Down Expand Up @@ -76,5 +81,6 @@ serverless client remove
- [amsross](https://github.com/amsross)
- [pradel](https://github.com/pradel)
- [daguix](https://github.com/daguix)
- [evanseeds](https://github.com/evanseeds)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for saving me the work 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I try to be thorough 😅


Forked from the [**serverless-client-s3**](https://github.com/serverless/serverless-client-s3/)
12 changes: 10 additions & 2 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,19 @@ class Client {
function configureBucket() {
this.serverless.cli.log(`Configuring website bucket ${this.bucketName}...`);

let indexDoc, errorDoc;
if (this.serverless.service.custom.spa) {
indexDoc = errorDoc = this.serverless.service.custom.client.indexDocument || 'index.html'
} else {
indexdoc = this.serverless.service.custom.client.indexDocument || 'index.html'
errorDoc = this.serverless.service.custom.client.errorDocument || 'error.html'
}

let params = {
Bucket: this.bucketName,
WebsiteConfiguration: {
IndexDocument: { Suffix: 'index.html' },
ErrorDocument: { Key: 'error.html' }
IndexDocument: { Suffix: indexDoc },
ErrorDocument: { Key: errorDoc }
}
};

Expand Down