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

Jump to bottom

update SSR render method, and introduce <:Head> #1024

Merged
merged 10 commits into from
Dec 14, 2017
Merged

update SSR render method, and introduce <:Head> #1024

merged 10 commits into from
Dec 14, 2017

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Dec 13, 2017 •
edited
Loading

This adds a <:Head> component, as per #1013, and updates the server-side render method so that it returns an object with html, css and head.

It also deprecates renderCss — we can remove this in v2.

@codecov-io
Copy link

codecov-io commented Dec 13, 2017 •
edited
Loading

Codecov Report

Merging #1024 into master will increase coverage by 0.06%.
The diff coverage is 90.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1024      +/-   ##
==========================================
+ Coverage    92.1%   92.16%   +0.06%     
==========================================
  Files         115      119       +4     
  Lines        4304     4444     +140     
  Branches     1373     1452      +79     
==========================================
+ Hits         3964     4096     +132     
+ Misses        147      146       -1     
- Partials      193      202       +9
Impacted Files Coverage Δ
src/generators/nodes/index.ts 100% <ø> (ø) ⬆️
...generators/server-side-rendering/visitors/index.ts 100% <ø> (ø) ⬆️
...nerators/server-side-rendering/visitors/Element.ts 90.32% <ø> (-1.57%) ⬇️
src/validate/html/validateHead.ts 0% <0%> (ø)
src/generators/nodes/RawMustacheTag.ts 100% <100%> (ø) ⬆️
src/validate/html/index.ts 97.05% <100%> (ø) ⬆️
src/generators/dom/Block.ts 98.19% <100%> (+0.01%) ⬆️
src/generators/nodes/Head.ts 100% <100%> (ø)
src/generators/nodes/Text.ts 100% <100%> (ø) ⬆️
src/generators/server-side-rendering/index.ts 96.29% <100%> (-0.63%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b961868...8a3898c. Read the comment docs.

@Rich-Harris
Copy link
Member Author

Upon reflection (with a little nudge from @vanesyan), we're probably better off with a <:Head> component that allows arbitrary tags. So the SSR render method should return { html, css, head } (or { body, css, head }? or is that weird?) instead of { html, css, title }, where head is just a chunk of HTML.

@Rich-Harris
Copy link
Member Author

I've now removed <:Document>, and replaced it with <:Head>. <title> isn't currently optimised, but that can be done at leisure.

One wrinkle: if you're SSR'ing <head> contents, and then hydrating on the client, you probably need a way to remove the SSR'd stuff, otherwise it'll double up. That's not something Svelte needs to worry about directly (Sapper can deal with it), but worth noting.

@Rich-Harris Rich-Harris changed the title update SSR render method, and introduce <:Document> update SSR render method, and introduce <:Head> Dec 14, 2017
@Rich-Harris Rich-Harris merged commit 394dec9 into master Dec 14, 2017
@Rich-Harris Rich-Harris deleted the gh-1013 branch December 14, 2017 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Rich-Harris @codecov-io