Skip to content
This repository was archived by the owner on May 17, 2019. It is now read-only.

Change server context shape, make SSRBodyTemplate optional #294

Closed
rtsao wants to merge 1 commit into from
Closed

Change server context shape, make SSRBodyTemplate optional #294

rtsao wants to merge 1 commit into from

Conversation

@rtsao
Copy link
Contributor

@rtsao rtsao commented Sep 17, 2018 •
edited
Loading

This PR further decouples fusion-core from fusion-cli.

Changes

  • Remove default SSRBodyTemplate, make it optional. For simulation tests, assertions should just use ctx.rendered, rather than assuming how ctx.body gets constructed.
  • Change default SSRDecider to check path prefix against ctx.assetBase rather than check extensions.
  • Change server context shape:
prefix
cdnUrl
nonce
useragent
element
- rootDir
- env
- assetPath
- preloadChunks
- webpackPublicPath
- syncChunks
- chunkUrlMap
+ assets
+ assetBase
+ criticalAssets
+ chunkAssetIndex

@rtsao rtsao added the breaking label Sep 17, 2018
revskill10
revskill10 reviewed Sep 18, 2018
View reviewed changes
src/plugins/ssr.js
// TODO(#46): this check should probably look at the asset manifest to ensure asset 404s are handled correctly
if (ctx.path.match(/\.(js|gif|jpg|png|pdf|json)$/)) return false;
if (ctx.path.startsWith(ctx.assetBase)) return false;
// The Accept header is a good proxy for whether SSR should happen
Copy link
Contributor

@revskill10 revskill10 Sep 18, 2018

Choose a reason for hiding this comment

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

We should use req.accepts(['html', 'json']) === 'html' instead ?

Copy link
Contributor

@revskill10 revskill10 Sep 18, 2018

Choose a reason for hiding this comment

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

Should we also provide whitelist extensions as default for user to override/extend ?

Copy link
Contributor

@revskill10 revskill10 Sep 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

What i want is to make sure default FusionJS website could be scraped and indexed by Bots.

@rtsao
Copy link
Contributor Author

rtsao commented Sep 20, 2018

Closing in favor of #299

@rtsao rtsao closed this Sep 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@revskill10 revskill10 revskill10 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

breaking

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rtsao @revskill10