From 2f5d755b5be7c04f208cbeb7d14beab2cf879344 Mon Sep 17 00:00:00 2001 From: Brian Takita Date: Sun, 26 May 2019 02:11:54 -0400 Subject: [PATCH 1/5] Additional detail to jsdocs for writable, readable, & derived Fixes https://github.com/sveltejs/svelte/issues/2867 --- src/store.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/store.ts b/src/store.ts index 361632b61d..a957ea66aa 100644 --- a/src/store.ts +++ b/src/store.ts @@ -45,8 +45,8 @@ type SubscribeInvalidateTuple = [Subscriber, Invalidater]; /** * Creates a `Readable` store that allows reading by subscription. - * @param value initial value - * @param start start and stop notifications for subscriptions + * @param [value] initial value + * @param [start] start and stop notifications for subscriptions */ export function readable(value: T, start: StartStopNotifier): Readable { return { @@ -56,8 +56,8 @@ export function readable(value: T, start: StartStopNotifier): Readable /** * Create a `Writable` store that allows both updating and reading by subscription. - * @param value initial value - * @param start start and stop notifications for subscriptions + * @param [value] initial value + * @param {StartStopNotifier}[start] start and stop notifications for subscriptions */ export function writable(value: T, start: StartStopNotifier = noop): Writable { let stop: Unsubscriber; @@ -110,9 +110,9 @@ type StoresValues = T extends Readable ? U : /** * Derived value store by synchronizing one or more readable stores and * applying an aggregation function over its input values. - * @param stores input stores - * @param fn function callback that aggregates the values - * @param initial_value when used asynchronously + * @param {Stores} stores input stores + * @param {function(StoresValues,[Subscriber])|Unsubscriber|void}fn function callback that aggregates the values + * @param [initial_value] when used asynchronously */ export function derived( stores: S, From 6fc7001993e31b9d93f37bce4a6fae92fe691c00 Mon Sep 17 00:00:00 2001 From: Brian Takita Date: Sun, 26 May 2019 10:15:13 -0400 Subject: [PATCH 2/5] Apply suggestions from code review Co-Authored-By: Rich Harris --- src/store.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/store.ts b/src/store.ts index a957ea66aa..aff15ff2ca 100644 --- a/src/store.ts +++ b/src/store.ts @@ -45,8 +45,8 @@ type SubscribeInvalidateTuple = [Subscriber, Invalidater]; /** * Creates a `Readable` store that allows reading by subscription. - * @param [value] initial value - * @param [start] start and stop notifications for subscriptions + * @param value initial value + * @param {StartStopNotifier}start start and stop notifications for subscriptions */ export function readable(value: T, start: StartStopNotifier): Readable { return { From a98a70cf832b66f1414caa9156b30f8da3ec9830 Mon Sep 17 00:00:00 2001 From: Brian Takita Date: Sun, 26 May 2019 10:51:49 -0400 Subject: [PATCH 3/5] jsdoc: `derived` second argument * optional first argument is a Stores type * optional second argument is a function that takes a single argument * has a return value --- src/store.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/store.ts b/src/store.ts index aff15ff2ca..e7db228401 100644 --- a/src/store.ts +++ b/src/store.ts @@ -56,8 +56,8 @@ export function readable(value: T, start: StartStopNotifier): Readable /** * Create a `Writable` store that allows both updating and reading by subscription. - * @param [value] initial value - * @param {StartStopNotifier}[start] start and stop notifications for subscriptions + * @param {*=}value initial value + * @param {StartStopNotifier=}start start and stop notifications for subscriptions */ export function writable(value: T, start: StartStopNotifier = noop): Writable { let stop: Unsubscriber; @@ -111,8 +111,8 @@ type StoresValues = T extends Readable ? U : * Derived value store by synchronizing one or more readable stores and * applying an aggregation function over its input values. * @param {Stores} stores input stores - * @param {function(StoresValues,[Subscriber])|Unsubscriber|void}fn function callback that aggregates the values - * @param [initial_value] when used asynchronously + * @param {function(Stores=, function(*)=):*}fn function callback that aggregates the values + * @param {*=}initial_value when used asynchronously */ export function derived( stores: S, From 04dce6816851c08a4dd6da03d50145b4c099b7e6 Mon Sep 17 00:00:00 2001 From: Viet Truong Date: Sun, 26 May 2019 16:58:46 -0700 Subject: [PATCH 4/5] publish dist in order to get types --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index b7214c9254..66f433e675 100644 --- a/package.json +++ b/package.json @@ -5,6 +5,7 @@ "module": "index.mjs", "main": "index", "files": [ + "dist", "compiler.js", "register.js", "index.*", From f0831202d9243f9f216693308b20b95add1df971 Mon Sep 17 00:00:00 2001 From: Colin Casey Date: Sun, 26 May 2019 23:44:24 -0300 Subject: [PATCH 5/5] Omits readonly attributes from SSR code * move `is_readonly` into the common `Binding` AST class * prevents the following bindings from being emitted into the SSR code: * `bind:clientWidth` * `bind:clientHeight` * `bind:offsetWidth` * `bind:offsetHeight` * `bind:duration` * `bind:buffered` * `bind:seekable` * `bind:played` * `bind:value` (only for `input` with `type=file`) --- src/compile/nodes/Binding.ts | 22 ++++++++++ src/compile/nodes/shared/Node.ts | 2 +- .../render-dom/wrappers/Element/Binding.ts | 18 +------- src/compile/render-ssr/handlers/Element.ts | 4 ++ .../samples/bindings-readonly/_expected.html | 4 ++ .../samples/bindings-readonly/main.svelte | 44 +++++++++++++++++++ 6 files changed, 77 insertions(+), 17 deletions(-) create mode 100644 test/server-side-rendering/samples/bindings-readonly/_expected.html create mode 100644 test/server-side-rendering/samples/bindings-readonly/main.svelte diff --git a/src/compile/nodes/Binding.ts b/src/compile/nodes/Binding.ts index 0d666a543f..e099f4405a 100644 --- a/src/compile/nodes/Binding.ts +++ b/src/compile/nodes/Binding.ts @@ -3,6 +3,15 @@ import get_object from '../utils/get_object'; import Expression from './shared/Expression'; import Component from '../Component'; import TemplateScope from './shared/TemplateScope'; +import {dimensions} from "../../utils/patterns"; + +// TODO this should live in a specific binding +const read_only_media_attributes = new Set([ + 'duration', + 'buffered', + 'seekable', + 'played' +]); export default class Binding extends Node { type: 'Binding'; @@ -11,6 +20,7 @@ export default class Binding extends Node { is_contextual: boolean; obj: string; prop: string; + is_readonly: boolean; constructor(component: Component, parent, scope: TemplateScope, info) { super(component, parent, scope, info); @@ -64,5 +74,17 @@ export default class Binding extends Node { this.obj = obj; this.prop = prop; + + const type = parent.get_static_attribute_value('type'); + + this.is_readonly = ( + dimensions.test(this.name) || + (parent.is_media_node && parent.is_media_node() && read_only_media_attributes.has(this.name)) || + (parent.name === 'input' && type === 'file') // TODO others? + ); + } + + is_readonly_media_attribute() { + return read_only_media_attributes.has(this.name); } } diff --git a/src/compile/nodes/shared/Node.ts b/src/compile/nodes/shared/Node.ts index b647e3a158..f27e4ceb62 100644 --- a/src/compile/nodes/shared/Node.ts +++ b/src/compile/nodes/shared/Node.ts @@ -47,7 +47,7 @@ export default class Node { } get_static_attribute_value(name: string) { - const attribute = this.attributes.find( + const attribute = this.attributes && this.attributes.find( (attr: Attribute) => attr.type === 'Attribute' && attr.name.toLowerCase() === name ); diff --git a/src/compile/render-dom/wrappers/Element/Binding.ts b/src/compile/render-dom/wrappers/Element/Binding.ts index a34a9466fc..b88479ad52 100644 --- a/src/compile/render-dom/wrappers/Element/Binding.ts +++ b/src/compile/render-dom/wrappers/Element/Binding.ts @@ -9,14 +9,6 @@ import flatten_reference from '../../../utils/flatten_reference'; import EachBlock from '../../../nodes/EachBlock'; import { Node as INode } from '../../../../interfaces'; -// TODO this should live in a specific binding -const read_only_media_attributes = new Set([ - 'duration', - 'buffered', - 'seekable', - 'played' -]); - function get_tail(node: INode) { const end = node.end; while (node.type === 'MemberExpression') node = node.object; @@ -74,13 +66,7 @@ export default class BindingWrapper { this.snippet = this.node.expression.render(block); - const type = parent.node.get_static_attribute_value('type'); - - this.is_readonly = ( - dimensions.test(this.node.name) || - (parent.node.is_media_node() && read_only_media_attributes.has(this.node.name)) || - (parent.node.name === 'input' && type === 'file') // TODO others? - ); + this.is_readonly = this.node.is_readonly; this.needs_lock = this.node.name === 'currentTime'; // TODO others? } @@ -101,7 +87,7 @@ export default class BindingWrapper { } is_readonly_media_attribute() { - return read_only_media_attributes.has(this.node.name); + return this.node.is_readonly_media_attribute() } render(block: Block, lock: string) { diff --git a/src/compile/render-ssr/handlers/Element.ts b/src/compile/render-ssr/handlers/Element.ts index fc6bc5b969..5ed6875dec 100644 --- a/src/compile/render-ssr/handlers/Element.ts +++ b/src/compile/render-ssr/handlers/Element.ts @@ -140,6 +140,10 @@ export default function(node: Element, renderer: Renderer, options: RenderOption node.bindings.forEach(binding => { const { name, expression } = binding; + if (binding.is_readonly) { + return; + } + if (name === 'group') { // TODO server-render group bindings } else { diff --git a/test/server-side-rendering/samples/bindings-readonly/_expected.html b/test/server-side-rendering/samples/bindings-readonly/_expected.html new file mode 100644 index 0000000000..af678acde0 --- /dev/null +++ b/test/server-side-rendering/samples/bindings-readonly/_expected.html @@ -0,0 +1,4 @@ +
+ + + diff --git a/test/server-side-rendering/samples/bindings-readonly/main.svelte b/test/server-side-rendering/samples/bindings-readonly/main.svelte new file mode 100644 index 0000000000..3ff98a3ead --- /dev/null +++ b/test/server-side-rendering/samples/bindings-readonly/main.svelte @@ -0,0 +1,44 @@ + + +
+ + + + + +