From 621333ea988a823cb6e3a324ed4c459a50771cae Mon Sep 17 00:00:00 2001 From: Conduitry Date: Mon, 13 Jan 2020 21:41:16 -0500 Subject: [PATCH 1/5] only add markers to head elements in SSR with hydratable=true (#4258) --- src/compiler/compile/render_ssr/handlers/Element.ts | 2 +- src/compiler/compile/render_ssr/handlers/Title.ts | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/compiler/compile/render_ssr/handlers/Element.ts b/src/compiler/compile/render_ssr/handlers/Element.ts index 4c1eca8a9d37..e0982a0415b6 100644 --- a/src/compiler/compile/render_ssr/handlers/Element.ts +++ b/src/compiler/compile/render_ssr/handlers/Element.ts @@ -124,7 +124,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption } }); - if (options.head_id) { + if (options.hydratable && options.head_id) { renderer.add_string(` data-svelte="${options.head_id}"`); } diff --git a/src/compiler/compile/render_ssr/handlers/Title.ts b/src/compiler/compile/render_ssr/handlers/Title.ts index e93ae13d6601..a3f271ab1b4b 100644 --- a/src/compiler/compile/render_ssr/handlers/Title.ts +++ b/src/compiler/compile/render_ssr/handlers/Title.ts @@ -5,7 +5,11 @@ import { x } from 'code-red'; export default function(node: Title, renderer: Renderer, options: RenderOptions) { renderer.push(); - renderer.add_string(``); + renderer.add_string('<title'); + if (options.hydratable && options.head_id) { + renderer.add_string(` data-svelte="${options.head_id}"`); + } + renderer.add_string('>'); renderer.render(node.children, options); From 84e0ef58a4df082ab0033b59a4bd5e3582b2d305 Mon Sep 17 00:00:00 2001 From: Conduitry <git@chor.date> Date: Mon, 13 Jan 2020 22:00:43 -0500 Subject: [PATCH 2/5] update and tidy tests --- test/helpers.js | 6 ++++ test/runtime/index.js | 7 ++--- test/server-side-rendering/index.js | 28 +++++++++---------- .../head-meta-hydrate-duplicate/_config.js | 5 ++++ .../head-multiple-title/_expected-head.html | 2 +- .../samples/head-title/_expected-head.html | 2 +- 6 files changed, 28 insertions(+), 22 deletions(-) create mode 100644 test/server-side-rendering/samples/head-meta-hydrate-duplicate/_config.js diff --git a/test/helpers.js b/test/helpers.js index 5e9428243b1a..a764d43f9646 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -45,6 +45,12 @@ export function tryToReadFile(file) { } } +export function cleanRequireCache() { + Object.keys(require.cache) + .filter(x => x.endsWith('.svelte')) + .forEach(file => delete require.cache[file]); +} + const virtualConsole = new jsdom.VirtualConsole(); virtualConsole.sendTo(console); diff --git a/test/runtime/index.js b/test/runtime/index.js index df02cabcb4b9..f070eb818551 100644 --- a/test/runtime/index.js +++ b/test/runtime/index.js @@ -10,6 +10,7 @@ import { showOutput, loadConfig, loadSvelte, + cleanRequireCache, env, setupHtmlEqual, mkdirp @@ -79,11 +80,7 @@ describe("runtime", () => { compileOptions.immutable = config.immutable; compileOptions.accessors = 'accessors' in config ? config.accessors : true; - Object.keys(require.cache) - .filter(x => x.endsWith('.svelte')) - .forEach(file => { - delete require.cache[file]; - }); + cleanRequireCache(); let mod; let SvelteComponent; diff --git a/test/server-side-rendering/index.js b/test/server-side-rendering/index.js index a56a4ddaed8b..f65637a1db7f 100644 --- a/test/server-side-rendering/index.js +++ b/test/server-side-rendering/index.js @@ -9,6 +9,7 @@ import { loadSvelte, setupHtmlEqual, tryToLoadJson, + cleanRequireCache, shouldUpdateExpected, mkdirp } from "../helpers.js"; @@ -27,11 +28,6 @@ let compile = null; describe("ssr", () => { before(() => { - require("../../register")({ - extensions: ['.svelte', '.html'], - sveltePath - }); - compile = loadSvelte(true).compile; return setupHtmlEqual(); @@ -40,9 +36,11 @@ describe("ssr", () => { fs.readdirSync(`${__dirname}/samples`).forEach(dir => { if (dir[0] === ".") return; + const config = loadConfig(`${__dirname}/samples/${dir}/_config.js`); + // add .solo to a sample directory name to only run that test, or // .show to always show the output. or both - const solo = /\.solo/.test(dir); + const solo = config.solo || /\.solo/.test(dir); const show = /\.show/.test(dir); if (solo && process.env.CI) { @@ -51,6 +49,13 @@ describe("ssr", () => { (solo ? it.only : it)(dir, () => { dir = path.resolve(`${__dirname}/samples`, dir); + + cleanRequireCache(); + + const compileOptions = { sveltePath, ...config.compileOptions }; + + require("../../register")(compileOptions); + try { const Component = require(`${dir}/main.svelte`).default; @@ -133,18 +138,11 @@ describe("ssr", () => { (config.skip ? it.skip : solo ? it.only : it)(dir, () => { const cwd = path.resolve("test/runtime/samples", dir); - Object.keys(require.cache) - .filter(x => x.endsWith('.svelte')) - .forEach(file => { - delete require.cache[file]; - }); + cleanRequireCache(); delete global.window; - const compileOptions = Object.assign({ sveltePath }, config.compileOptions, { - generate: 'ssr', - format: 'cjs' - }); + const compileOptions = { sveltePath, ...config.compileOptions }; require("../../register")(compileOptions); diff --git a/test/server-side-rendering/samples/head-meta-hydrate-duplicate/_config.js b/test/server-side-rendering/samples/head-meta-hydrate-duplicate/_config.js new file mode 100644 index 000000000000..ae9b250f8657 --- /dev/null +++ b/test/server-side-rendering/samples/head-meta-hydrate-duplicate/_config.js @@ -0,0 +1,5 @@ +export default { + compileOptions: { + hydratable: true + } +}; diff --git a/test/server-side-rendering/samples/head-multiple-title/_expected-head.html b/test/server-side-rendering/samples/head-multiple-title/_expected-head.html index 71475508396f..af5c5feba460 100644 --- a/test/server-side-rendering/samples/head-multiple-title/_expected-head.html +++ b/test/server-side-rendering/samples/head-multiple-title/_expected-head.html @@ -1 +1 @@ -<title data-svelte="svelte-1csszk6">B \ No newline at end of file +B \ No newline at end of file diff --git a/test/server-side-rendering/samples/head-title/_expected-head.html b/test/server-side-rendering/samples/head-title/_expected-head.html index 6e73e671e6b0..7d696352f9eb 100644 --- a/test/server-side-rendering/samples/head-title/_expected-head.html +++ b/test/server-side-rendering/samples/head-title/_expected-head.html @@ -1 +1 @@ -a custom title \ No newline at end of file +a custom title \ No newline at end of file From 6fe40402f617a53e7884ae27f3f248f9a6eb3f90 Mon Sep 17 00:00:00 2001 From: Conduitry Date: Mon, 13 Jan 2020 22:03:15 -0500 Subject: [PATCH 3/5] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32012876a3e3..480c145a9557 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Svelte changelog +## Unreleased + +* Only attach SSR mode markers to a component's `` elements when compiling with `hydratable: true` ([#4258](https://github.com/sveltejs/svelte/issues/4258)) + ## 3.17.0 * Remove old `` elements during hydration so they aren't duplicated ([#1607](https://github.com/sveltejs/svelte/issues/1607)) From 0981da3c3b416fcc095776604771136d670c0afc Mon Sep 17 00:00:00 2001 From: Conduitry Date: Mon, 13 Jan 2020 22:10:19 -0500 Subject: [PATCH 4/5] fix test tooling regression --- test/server-side-rendering/index.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/server-side-rendering/index.js b/test/server-side-rendering/index.js index f65637a1db7f..ee1319845dfe 100644 --- a/test/server-side-rendering/index.js +++ b/test/server-side-rendering/index.js @@ -52,7 +52,12 @@ describe("ssr", () => { cleanRequireCache(); - const compileOptions = { sveltePath, ...config.compileOptions }; + const compileOptions = { + sveltePath, + ...config.compileOptions, + generate: 'ssr', + format: 'cjs' + }; require("../../register")(compileOptions); @@ -142,7 +147,12 @@ describe("ssr", () => { delete global.window; - const compileOptions = { sveltePath, ...config.compileOptions }; + const compileOptions = { + sveltePath, + ...config.compileOptions, + generate: 'ssr', + format: 'cjs' + }; require("../../register")(compileOptions); From e72be5db53bdac5e5c07a29e3b2e53516e80b83c Mon Sep 17 00:00:00 2001 From: Conduitry Date: Tue, 14 Jan 2020 12:06:20 -0500 Subject: [PATCH 5/5] update docs --- site/content/docs/03-run-time.md | 2 +- site/content/docs/04-compile-time.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/site/content/docs/03-run-time.md b/site/content/docs/03-run-time.md index e1509cf4d1c7..c75ec694d932 100644 --- a/site/content/docs/03-run-time.md +++ b/site/content/docs/03-run-time.md @@ -887,7 +887,7 @@ Existing children of `target` are left where they are. --- -The `hydrate` option instructs Svelte to upgrade existing DOM (usually from server-side rendering) rather than creating new elements. It will only work if the component was compiled with the [`hydratable: true` option](docs#svelte_compile). +The `hydrate` option instructs Svelte to upgrade existing DOM (usually from server-side rendering) rather than creating new elements. It will only work if the component was compiled with the [`hydratable: true` option](docs#svelte_compile). Hydration of `` elements only works properly if the server-side rendering code was also compiled with `hydratable: true`, which adds a marker to each element in the `` so that the component knows which elements it's responsible for removing during hydration. Whereas children of `target` are normally left alone, `hydrate: true` will cause any children to be removed. For that reason, the `anchor` option cannot be used alongside `hydrate: true`. diff --git a/site/content/docs/04-compile-time.md b/site/content/docs/04-compile-time.md index e9126ccf1316..f753b2fcfeda 100644 --- a/site/content/docs/04-compile-time.md +++ b/site/content/docs/04-compile-time.md @@ -68,7 +68,7 @@ The following options can be passed to the compiler. None are required: | `generate` | `"dom"` | If `"dom"`, Svelte emits a JavaScript class for mounting to the DOM. If `"ssr"`, Svelte emits an object with a `render` method suitable for server-side rendering. If `false`, no JavaScript or CSS is returned; just metadata. | `dev` | `false` | If `true`, causes extra code to be added to components that will perform runtime checks and provide debugging information during development. | `immutable` | `false` | If `true`, tells the compiler that you promise not to mutate any objects. This allows it to be less conservative about checking whether values have changed. -| `hydratable` | `false` | If `true`, enables the `hydrate: true` runtime option, which allows a component to upgrade existing DOM rather than creating new DOM from scratch. +| `hydratable` | `false` | If `true` when generating DOM code, enables the `hydrate: true` runtime option, which allows a component to upgrade existing DOM rather than creating new DOM from scratch. When generating SSR code, this adds markers to `` elements so that hydration knows which to replace. | `legacy` | `false` | If `true`, generates code that will work in IE9 and IE10, which don't support things like `element.dataset`. | `accessors` | `false` | If `true`, getters and setters will be created for the component's props. If `false`, they will only be created for readonly exported values (i.e. those declared with `const`, `class` and `function`). If compiling with `customElement: true` this option defaults to `true`. | `customElement` | `false` | If `true`, tells the compiler to generate a custom element constructor instead of a regular Svelte component.