From def1890f4ff0cccb9573bfc79984fbe1258ed318 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 11 Apr 2023 02:25:53 -0700 Subject: [PATCH 01/20] chore: bump @jridgewell/sourcemap-codec (#8458) --- package-lock.json | 14 +++++++------- package.json | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2c894b1605..3f32f9633f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "license": "MIT", "devDependencies": { "@ampproject/remapping": "^0.3.0", - "@jridgewell/sourcemap-codec": "^1.4.14", + "@jridgewell/sourcemap-codec": "^1.4.15", "@rollup/plugin-commonjs": "^11.0.0", "@rollup/plugin-json": "^6.0.0", "@rollup/plugin-node-resolve": "^11.2.1", @@ -184,9 +184,9 @@ } }, "node_modules/@jridgewell/sourcemap-codec": { - "version": "1.4.14", - "resolved": "https://registry.npmjs.org/@jridgewell/sourcemap-codec/-/sourcemap-codec-1.4.14.tgz", - "integrity": "sha512-XPSJHWmi394fuUuzDnGz1wiKqWfo1yXecHQMRf2l6hztTO+nPru658AyDngaBe7isIxEkRsPR3FZh+s7iVa4Uw==", + "version": "1.4.15", + "resolved": "https://registry.npmjs.org/@jridgewell/sourcemap-codec/-/sourcemap-codec-1.4.15.tgz", + "integrity": "sha512-eF2rxCRulEKXHTRiDrDy6erMYWqNw4LPdQ8UQA4huuxaQsVeRPFl2oM8oDGxMFhJUWZf9McpLtJasDDZb/Bpeg==", "dev": true }, "node_modules/@nodelib/fs.scandir": { @@ -5499,9 +5499,9 @@ "dev": true }, "@jridgewell/sourcemap-codec": { - "version": "1.4.14", - "resolved": "https://registry.npmjs.org/@jridgewell/sourcemap-codec/-/sourcemap-codec-1.4.14.tgz", - "integrity": "sha512-XPSJHWmi394fuUuzDnGz1wiKqWfo1yXecHQMRf2l6hztTO+nPru658AyDngaBe7isIxEkRsPR3FZh+s7iVa4Uw==", + "version": "1.4.15", + "resolved": "https://registry.npmjs.org/@jridgewell/sourcemap-codec/-/sourcemap-codec-1.4.15.tgz", + "integrity": "sha512-eF2rxCRulEKXHTRiDrDy6erMYWqNw4LPdQ8UQA4huuxaQsVeRPFl2oM8oDGxMFhJUWZf9McpLtJasDDZb/Bpeg==", "dev": true }, "@nodelib/fs.scandir": { diff --git a/package.json b/package.json index dc44115e39..25aafb817a 100644 --- a/package.json +++ b/package.json @@ -120,7 +120,7 @@ "homepage": "https://svelte.dev", "devDependencies": { "@ampproject/remapping": "^0.3.0", - "@jridgewell/sourcemap-codec": "^1.4.14", + "@jridgewell/sourcemap-codec": "^1.4.15", "@rollup/plugin-commonjs": "^11.0.0", "@rollup/plugin-json": "^6.0.0", "@rollup/plugin-node-resolve": "^11.2.1", From 3a7685fef554f0ae2480146eb7c2d0ec944b28ac Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 11 Apr 2023 11:44:19 +0200 Subject: [PATCH 02/20] fix: special-case width/height attribute during spread (#8412) fixes #6752 --------- Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Tan Li Hau --- src/runtime/internal/dom.ts | 11 ++++++++++- .../samples/spread-width-height-attributes/_config.js | 4 ++++ .../spread-width-height-attributes/main.svelte | 1 + 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 test/runtime/samples/spread-width-height-attributes/_config.js create mode 100644 test/runtime/samples/spread-width-height-attributes/main.svelte diff --git a/src/runtime/internal/dom.ts b/src/runtime/internal/dom.ts index 34049b3580..8a78accb50 100644 --- a/src/runtime/internal/dom.ts +++ b/src/runtime/internal/dom.ts @@ -306,6 +306,15 @@ export function attr(node: Element, attribute: string, value?: string) { else if (node.getAttribute(attribute) !== value) node.setAttribute(attribute, value); } +/** + * List of attributes that should always be set through the attr method, + * because updating them through the property setter doesn't work reliably. + * In the example of `width`/`height`, the problem is that the setter only + * accepts numeric values, but the attribute can also be set to a string like `50%`. + * If this list becomes too big, rethink this approach. + */ +const always_set_through_set_attribute = ['width', 'height']; + export function set_attributes(node: Element & ElementCSSInlineStyle, attributes: { [x: string]: string }) { // @ts-ignore const descriptors = Object.getOwnPropertyDescriptors(node.__proto__); @@ -316,7 +325,7 @@ export function set_attributes(node: Element & ElementCSSInlineStyle, attributes node.style.cssText = attributes[key]; } else if (key === '__value') { (node as any).value = node[key] = attributes[key]; - } else if (descriptors[key] && descriptors[key].set) { + } else if (descriptors[key] && descriptors[key].set && always_set_through_set_attribute.indexOf(key) === -1) { node[key] = attributes[key]; } else { attr(node, key, attributes[key]); diff --git a/test/runtime/samples/spread-width-height-attributes/_config.js b/test/runtime/samples/spread-width-height-attributes/_config.js new file mode 100644 index 0000000000..cf2dc7efde --- /dev/null +++ b/test/runtime/samples/spread-width-height-attributes/_config.js @@ -0,0 +1,4 @@ +export default { + // https://github.com/sveltejs/svelte/issues/6752 + html: '' +}; diff --git a/test/runtime/samples/spread-width-height-attributes/main.svelte b/test/runtime/samples/spread-width-height-attributes/main.svelte new file mode 100644 index 0000000000..b91b008457 --- /dev/null +++ b/test/runtime/samples/spread-width-height-attributes/main.svelte @@ -0,0 +1 @@ + From 0adc09da9714bb0fcc7fafdbee569ea7cad4fae5 Mon Sep 17 00:00:00 2001 From: Cymaera <69355340+TheCymaera@users.noreply.github.com> Date: Tue, 11 Apr 2023 18:17:58 +0800 Subject: [PATCH 03/20] feat: add support for resize observer bindings (#8022) Implements ResizeObserver bindings: #5524 (comment) Continuation of: #5963 Related to #7583 --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- elements/index.d.ts | 5 ++ src/compiler/compile/nodes/Binding.ts | 3 +- src/compiler/compile/nodes/Element.ts | 7 +- .../render_dom/wrappers/Element/Binding.ts | 8 ++- .../render_dom/wrappers/Element/index.ts | 32 +++++++-- src/compiler/utils/patterns.ts | 6 ++ .../internal/ResizeObserverSingleton.ts | 67 +++++++++++++++++++ src/runtime/internal/dom.ts | 8 ++- 8 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 src/runtime/internal/ResizeObserverSingleton.ts diff --git a/elements/index.d.ts b/elements/index.d.ts index 7595d767bf..ac32ae94c3 100644 --- a/elements/index.d.ts +++ b/elements/index.d.ts @@ -546,6 +546,11 @@ export interface HTMLAttributes extends AriaAttributes, D */ 'bind:innerText'?: string | undefined | null; + readonly 'bind:contentRect'?: DOMRectReadOnly | undefined | null; + readonly 'bind:contentBoxSize'?: Array<{ blockSize: number; inlineSize: number }> | undefined | null; // TODO make this ResizeObserverSize once we require TS>=4.4 + readonly 'bind:borderBoxSize'?: Array<{ blockSize: number; inlineSize: number }> | undefined | null; // TODO make this ResizeObserverSize once we require TS>=4.4 + readonly 'bind:devicePixelContentBoxSize'?: Array<{ blockSize: number; inlineSize: number }> | undefined | null; // TODO make this ResizeObserverSize once we require TS>=4.4 + // SvelteKit 'data-sveltekit-keepfocus'?: true | '' | 'off' | undefined | null; 'data-sveltekit-noscroll'?: true | '' | 'off' | undefined | null; diff --git a/src/compiler/compile/nodes/Binding.ts b/src/compiler/compile/nodes/Binding.ts index 0c29f7ec67..303506222f 100644 --- a/src/compiler/compile/nodes/Binding.ts +++ b/src/compiler/compile/nodes/Binding.ts @@ -3,7 +3,7 @@ import get_object from '../utils/get_object'; import Expression from './shared/Expression'; import Component from '../Component'; import TemplateScope from './shared/TemplateScope'; -import { regex_dimensions } from '../../utils/patterns'; +import { regex_dimensions, regex_box_size } from '../../utils/patterns'; import { Node as ESTreeNode } from 'estree'; import { TemplateNode } from '../../interfaces'; import Element from './Element'; @@ -92,6 +92,7 @@ export default class Binding extends Node { this.is_readonly = regex_dimensions.test(this.name) || + regex_box_size.test(this.name) || (isElement(parent) && ((parent.is_media_node() && read_only_media_attributes.has(this.name)) || (parent.name === 'input' && type === 'file')) /* TODO others? */); diff --git a/src/compiler/compile/nodes/Element.ts b/src/compiler/compile/nodes/Element.ts index 1678ea1caa..2410904d63 100644 --- a/src/compiler/compile/nodes/Element.ts +++ b/src/compiler/compile/nodes/Element.ts @@ -12,7 +12,7 @@ import Text from './Text'; import { namespaces } from '../../utils/namespaces'; import map_children from './shared/map_children'; import { is_name_contenteditable, get_contenteditable_attr } from '../utils/contenteditable'; -import { regex_dimensions, regex_starts_with_newline, regex_non_whitespace_character } from '../../utils/patterns'; +import { regex_dimensions, regex_starts_with_newline, regex_non_whitespace_character, regex_box_size } from '../../utils/patterns'; import fuzzymatch from '../../utils/fuzzymatch'; import list from '../../utils/list'; import Let from './Let'; @@ -1090,7 +1090,10 @@ export default class Element extends Node { } else if (contenteditable && !contenteditable.is_static) { return component.error(contenteditable, compiler_errors.dynamic_contenteditable_attribute); } - } else if (name !== 'this') { + } else if ( + name !== 'this' && + !regex_box_size.test(name) + ) { return component.error(binding, compiler_errors.invalid_binding(binding.name)); } }); diff --git a/src/compiler/compile/render_dom/wrappers/Element/Binding.ts b/src/compiler/compile/render_dom/wrappers/Element/Binding.ts index 642e4694b1..01da1f0e12 100644 --- a/src/compiler/compile/render_dom/wrappers/Element/Binding.ts +++ b/src/compiler/compile/render_dom/wrappers/Element/Binding.ts @@ -11,6 +11,7 @@ import { Node, Identifier } from 'estree'; import add_to_set from '../../../utils/add_to_set'; import mark_each_block_bindings from '../shared/mark_each_block_bindings'; import handle_select_value_binding from './handle_select_value_binding'; +import { regex_box_size } from '../../../../utils/patterns'; export default class BindingWrapper { node: Binding; @@ -455,7 +456,12 @@ function get_value_from_dom( return x`$$value`; } - // node.name === 'input' && node.get_static_attribute_value('type') === 'range' }, + // resize events { event_names: ['elementresize'], filter: (_node: Element, name: string) => regex_dimensions.test(name) }, + { + event_names: ['elementresizecontentbox'], + filter: (_node: Element, name: string) => + regex_content_rect.test(name) ?? regex_content_box_size.test(name) + }, + + { + event_names: ['elementresizeborderbox'], + filter: (_node: Element, name: string) => + regex_border_box_size.test(name) + }, + + { + event_names: ['elementresizedevicepixelcontentbox'], + filter: (_node: Element, name: string) => + regex_device_pixel_content_box_size.test(name) + }, // media events { event_names: ['timeupdate'], @@ -747,13 +765,19 @@ export default class ElementWrapper extends Wrapper { `); binding_group.events.forEach(name => { - if (name === 'elementresize') { - // special case + const resizeListenerFunctions = { + elementresize: 'add_iframe_resize_listener', + elementresizecontentbox: 'resize_observer_content_box.observe', + elementresizeborderbox: 'resize_observer_border_box.observe', + elementresizedevicepixelcontentbox: 'resize_observer_device_pixel_content_box.observe' + }; + + if (name in resizeListenerFunctions) { const resize_listener = block.get_unique_name(`${this.var.name}_resize_listener`); block.add_variable(resize_listener); block.chunks.mount.push( - b`${resize_listener} = @add_resize_listener(${this.var}, ${callee}.bind(${this.var}));` + b`${resize_listener} = @${resizeListenerFunctions[name]}(${this.var}, ${callee}.bind(${this.var}));` ); block.chunks.destroy.push( diff --git a/src/compiler/utils/patterns.ts b/src/compiler/utils/patterns.ts index 9429d47227..f0fb08c7c9 100644 --- a/src/compiler/utils/patterns.ts +++ b/src/compiler/utils/patterns.ts @@ -22,3 +22,9 @@ export const regex_ends_with_underscore = /_$/; export const regex_invalid_variable_identifier_characters = /[^a-zA-Z0-9_$]/g; export const regex_dimensions = /^(?:offset|client)(?:Width|Height)$/; + +export const regex_content_rect = /^(?:contentRect)$/; +export const regex_content_box_size = /^(?:contentBoxSize)$/; +export const regex_border_box_size = /^(?:borderBoxSize)$/; +export const regex_device_pixel_content_box_size = /^(?:devicePixelContentBoxSize)$/; +export const regex_box_size = /^(?:contentRect|contentBoxSize|borderBoxSize|devicePixelContentBoxSize)$/; diff --git a/src/runtime/internal/ResizeObserverSingleton.ts b/src/runtime/internal/ResizeObserverSingleton.ts new file mode 100644 index 0000000000..6d1e5b567b --- /dev/null +++ b/src/runtime/internal/ResizeObserverSingleton.ts @@ -0,0 +1,67 @@ +/** + * Resize observer singleton. + * One listener per element only! + * https://groups.google.com/a/chromium.org/g/blink-dev/c/z6ienONUb5A/m/F5-VcUZtBAAJ + */ +export class ResizeObserverSingleton { + constructor(readonly options?: ResizeObserverOptions) {} + + observe(element: Element, listener: Listener) { + this._listeners.set(element, listener); + this._getObserver().observe(element, this.options); + return () => { + this._listeners.delete(element); + this._observer.unobserve(element); // this line can probably be removed + }; + } + + static readonly entries: WeakMap = 'WeakMap' in globalThis ? new WeakMap() : undefined; + + private readonly _listeners: WeakMap = 'WeakMap' in globalThis ? new WeakMap() : undefined; + private _observer?: ResizeObserver; + private _getObserver() { + return this._observer ?? (this._observer = new ResizeObserver((entries) => { + for (const entry of entries) { + ResizeObserverSingleton.entries.set(entry.target, entry); + this._listeners.get(entry.target)?.(entry); + } + })); + } +} + +type Listener = (entry: ResizeObserverEntry)=>any; + +// TODO: Remove this +interface ResizeObserverSize { + readonly blockSize: number; + readonly inlineSize: number; +} + +interface ResizeObserverEntry { + readonly borderBoxSize: readonly ResizeObserverSize[]; + readonly contentBoxSize: readonly ResizeObserverSize[]; + readonly contentRect: DOMRectReadOnly; + readonly devicePixelContentBoxSize: readonly ResizeObserverSize[]; + readonly target: Element; +} + +type ResizeObserverBoxOptions = 'border-box' | 'content-box' | 'device-pixel-content-box'; + +interface ResizeObserverOptions { + box?: ResizeObserverBoxOptions; +} + +interface ResizeObserver { + disconnect(): void; + observe(target: Element, options?: ResizeObserverOptions): void; + unobserve(target: Element): void; +} + +interface ResizeObserverCallback { + (entries: ResizeObserverEntry[], observer: ResizeObserver): void; +} + +declare let ResizeObserver: { + prototype: ResizeObserver; + new(callback: ResizeObserverCallback): ResizeObserver; +}; diff --git a/src/runtime/internal/dom.ts b/src/runtime/internal/dom.ts index 8a78accb50..4ffa9e4742 100644 --- a/src/runtime/internal/dom.ts +++ b/src/runtime/internal/dom.ts @@ -1,3 +1,4 @@ +import { ResizeObserverSingleton } from './ResizeObserverSingleton'; import { contenteditable_truthy_values, has_prop } from './utils'; // Track which nodes are claimed during hydration. Unclaimed nodes can then be removed from the DOM @@ -698,7 +699,7 @@ export function is_crossorigin() { return crossorigin; } -export function add_resize_listener(node: HTMLElement, fn: () => void) { +export function add_iframe_resize_listener(node: HTMLElement, fn: () => void) { const computed_style = getComputedStyle(node); if (computed_style.position === 'static') { @@ -746,6 +747,11 @@ export function add_resize_listener(node: HTMLElement, fn: () => void) { }; } +export const resize_observer_content_box = new ResizeObserverSingleton({ box: 'content-box' }); +export const resize_observer_border_box = new ResizeObserverSingleton({ box: 'border-box' }); +export const resize_observer_device_pixel_content_box = new ResizeObserverSingleton({ box: 'device-pixel-content-box' }); +export { ResizeObserverSingleton }; + export function toggle_class(element, name, toggle) { element.classList[toggle ? 'add' : 'remove'](name); } From 56351a3fabbc8ebd44723aa724b8050ab19a6dcd Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 11 Apr 2023 12:19:30 +0200 Subject: [PATCH 04/20] chore: update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5568dce50..ea9545aa65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Svelte changelog +## Unreleased + +* Handle `width`/`height` attributes when spreading ([#6752](https://github.com/sveltejs/svelte/issues/6752)) +* Add support for resize observer bindings (`
`) ([#8022](https://github.com/sveltejs/svelte/pull/8022)) + ## 3.58.0 * Add `bind:innerText` for `contenteditable` elements ([#3311](https://github.com/sveltejs/svelte/issues/3311)) From cd690e025bb2ad2e50cfc417d4b58408c779f080 Mon Sep 17 00:00:00 2001 From: James Scott-Brown Date: Tue, 11 Apr 2023 13:05:22 +0100 Subject: [PATCH 05/20] docs: clarify meaning of "this" in a comment (#8478) --- site/content/docs/02-component-format.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/content/docs/02-component-format.md b/site/content/docs/02-component-format.md index d9f7a35662..f1563a62c6 100644 --- a/site/content/docs/02-component-format.md +++ b/site/content/docs/02-component-format.md @@ -286,7 +286,7 @@ You cannot `export default`, since the default export is the component itself. + +
From 6bbae502f6d41ea37cc7c46585662bf6807561c0 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 18 Apr 2023 17:29:59 +0200 Subject: [PATCH 11/20] chore: Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ea9545aa65..d01782fa29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ * Handle `width`/`height` attributes when spreading ([#6752](https://github.com/sveltejs/svelte/issues/6752)) * Add support for resize observer bindings (`
`) ([#8022](https://github.com/sveltejs/svelte/pull/8022)) +* Update interpolated style directive properly when using spread ([#8438](https://github.com/sveltejs/svelte/issues/8438)) +* Ensure version is typed as `string` instead of the literal `__VERSION__` ([#8498](https://github.com/sveltejs/svelte/issues/8498)) ## 3.58.0 From 32153e318d7b5be0dae9334801dba31881f077bc Mon Sep 17 00:00:00 2001 From: xxkl1 <84455605+xxkl1@users.noreply.github.com> Date: Wed, 19 Apr 2023 21:21:24 +0800 Subject: [PATCH 12/20] fix: inline style value become undefined (#8517) fixes #8462 --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- src/runtime/internal/dom.ts | 4 ++-- .../samples/inline-style-become-undefined/_config.js | 11 +++++++++++ .../samples/inline-style-become-undefined/main.svelte | 9 +++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 test/runtime/samples/inline-style-become-undefined/_config.js create mode 100644 test/runtime/samples/inline-style-become-undefined/main.svelte diff --git a/src/runtime/internal/dom.ts b/src/runtime/internal/dom.ts index c025313c15..066fa6eb1c 100644 --- a/src/runtime/internal/dom.ts +++ b/src/runtime/internal/dom.ts @@ -307,7 +307,7 @@ export function attr(node: Element, attribute: string, value?: string) { else if (node.getAttribute(attribute) !== value) node.setAttribute(attribute, value); } -/** +/** * List of attributes that should always be set through the attr method, * because updating them through the property setter doesn't work reliably. * In the example of `width`/`height`, the problem is that the setter only @@ -641,7 +641,7 @@ export function set_input_type(input, type) { } export function set_style(node, key, value, important) { - if (value === null) { + if (value == null) { node.style.removeProperty(key); } else { node.style.setProperty(key, value, important ? 'important' : ''); diff --git a/test/runtime/samples/inline-style-become-undefined/_config.js b/test/runtime/samples/inline-style-become-undefined/_config.js new file mode 100644 index 0000000000..a2a0727efa --- /dev/null +++ b/test/runtime/samples/inline-style-become-undefined/_config.js @@ -0,0 +1,11 @@ +export default { + async test({ assert, target, window }) { + const div = target.querySelector('div'); + const click = new window.MouseEvent('click'); + + assert.htmlEqual(target.innerHTML, '
'); + await div.dispatchEvent(click); + await Promise.resolve(); + assert.htmlEqual(target.innerHTML, '
'); + } +}; diff --git a/test/runtime/samples/inline-style-become-undefined/main.svelte b/test/runtime/samples/inline-style-become-undefined/main.svelte new file mode 100644 index 0000000000..ee38934fc7 --- /dev/null +++ b/test/runtime/samples/inline-style-become-undefined/main.svelte @@ -0,0 +1,9 @@ + + +
From 6ba2f722518b3fb6904d6d566c3c1a00d61fe70a Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 19 Apr 2023 15:27:42 +0200 Subject: [PATCH 13/20] chore: Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d01782fa29..bc75b301a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ * Handle `width`/`height` attributes when spreading ([#6752](https://github.com/sveltejs/svelte/issues/6752)) * Add support for resize observer bindings (`
`) ([#8022](https://github.com/sveltejs/svelte/pull/8022)) * Update interpolated style directive properly when using spread ([#8438](https://github.com/sveltejs/svelte/issues/8438)) +* Remove style directive property when value is `undefined` ([#8462](https://github.com/sveltejs/svelte/issues/8462)) * Ensure version is typed as `string` instead of the literal `__VERSION__` ([#8498](https://github.com/sveltejs/svelte/issues/8498)) ## 3.58.0 From f064c39d5ff01a4e2aba71a3d3662c7300aba025 Mon Sep 17 00:00:00 2001 From: Nguyen Tran <88808276+ngtr6788@users.noreply.github.com> Date: Wed, 26 Apr 2023 03:18:22 -0400 Subject: [PATCH 14/20] fix: relax no-redundant-roles implementation (#8536) Deals with the no-redundant-roles part of #8529 There was an erroneous check which compares the element name with the current role. This fix brings no-redundant-roles closer to the original eslint-jsx implementation --- src/compiler/compile/nodes/Element.ts | 7 ++++--- .../validator/samples/a11y-no-redundant-roles/input.svelte | 6 +++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/compiler/compile/nodes/Element.ts b/src/compiler/compile/nodes/Element.ts index 2410904d63..44d84f7566 100644 --- a/src/compiler/compile/nodes/Element.ts +++ b/src/compiler/compile/nodes/Element.ts @@ -121,6 +121,7 @@ const a11y_implicit_semantics = new Map([ ['details', 'group'], ['dt', 'term'], ['fieldset', 'group'], + ['figure', 'figure'], ['form', 'form'], ['h1', 'heading'], ['h2', 'heading'], @@ -132,6 +133,7 @@ const a11y_implicit_semantics = new Map([ ['img', 'img'], ['li', 'listitem'], ['link', 'link'], + ['main', 'main'], ['menu', 'list'], ['meter', 'progressbar'], ['nav', 'navigation'], @@ -142,6 +144,7 @@ const a11y_implicit_semantics = new Map([ ['progress', 'progressbar'], ['section', 'region'], ['summary', 'button'], + ['table', 'table'], ['tbody', 'rowgroup'], ['textarea', 'textbox'], ['tfoot', 'rowgroup'], @@ -631,9 +634,7 @@ export default class Element extends Node { } // no-redundant-roles - const has_redundant_role = current_role === get_implicit_role(this.name, attribute_map); - - if (this.name === current_role || has_redundant_role) { + if (current_role === get_implicit_role(this.name, attribute_map)) { component.warn(attribute, compiler_warnings.a11y_no_redundant_roles(current_role)); } diff --git a/test/validator/samples/a11y-no-redundant-roles/input.svelte b/test/validator/samples/a11y-no-redundant-roles/input.svelte index 05525effb6..537d5c0fd3 100644 --- a/test/validator/samples/a11y-no-redundant-roles/input.svelte +++ b/test/validator/samples/a11y-no-redundant-roles/input.svelte @@ -41,4 +41,8 @@
-
\ No newline at end of file +
+ + + + From b7359c8361e476d1a5aba96b79c1749fec94cb3a Mon Sep 17 00:00:00 2001 From: xxkl1 <84455605+xxkl1@users.noreply.github.com> Date: Thu, 27 Apr 2023 22:17:42 +0800 Subject: [PATCH 15/20] feat: add window bind devicePixelRatio support (#8534) closes: #8285 add window bind devicePixelRatio support, change devicePixelRatio on window resize. --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- elements/index.d.ts | 1 + site/content/docs/03-template-syntax.md | 1 + src/compiler/compile/nodes/Window.ts | 1 + src/compiler/compile/render_dom/wrappers/Window.ts | 2 ++ .../runtime/samples/window-binding-resize/_config.js | 12 ++++++++++-- .../samples/window-binding-resize/main.svelte | 6 ++++-- .../samples/window-binding-invalid/errors.json | 2 +- 7 files changed, 20 insertions(+), 5 deletions(-) diff --git a/elements/index.d.ts b/elements/index.d.ts index ac32ae94c3..dd91d14654 100644 --- a/elements/index.d.ts +++ b/elements/index.d.ts @@ -1083,6 +1083,7 @@ export interface SvelteWindowAttributes extends HTMLAttributes { readonly 'bind:innerHeight'?: Window['innerHeight'] | undefined | null; readonly 'bind:outerWidth'?: Window['outerWidth'] | undefined | null; readonly 'bind:outerHeight'?: Window['outerHeight'] | undefined | null; + readonly 'bind:devicePixelRatio'?: Window['devicePixelRatio'] | undefined | null; 'bind:scrollX'?: Window['scrollX'] | undefined | null; 'bind:scrollY'?: Window['scrollY'] | undefined | null; readonly 'bind:online'?: Window['navigator']['onLine'] | undefined | null; diff --git a/site/content/docs/03-template-syntax.md b/site/content/docs/03-template-syntax.md index 170c303b85..27d3ca9987 100644 --- a/site/content/docs/03-template-syntax.md +++ b/site/content/docs/03-template-syntax.md @@ -1742,6 +1742,7 @@ You can also bind to the following properties: * `scrollX` * `scrollY` * `online` — an alias for `window.navigator.onLine` +* `devicePixelRatio` All except `scrollX` and `scrollY` are readonly. diff --git a/src/compiler/compile/nodes/Window.ts b/src/compiler/compile/nodes/Window.ts index c5bec0acd3..2f8a015d8a 100644 --- a/src/compiler/compile/nodes/Window.ts +++ b/src/compiler/compile/nodes/Window.ts @@ -17,6 +17,7 @@ const valid_bindings = [ 'outerHeight', 'scrollX', 'scrollY', + 'devicePixelRatio', 'online' ]; diff --git a/src/compiler/compile/render_dom/wrappers/Window.ts b/src/compiler/compile/render_dom/wrappers/Window.ts index c98af18268..9e58bbcaad 100644 --- a/src/compiler/compile/render_dom/wrappers/Window.ts +++ b/src/compiler/compile/render_dom/wrappers/Window.ts @@ -14,6 +14,7 @@ const associated_events = { innerHeight: 'resize', outerWidth: 'resize', outerHeight: 'resize', + devicePixelRatio: 'resize', scrollX: 'scroll', scrollY: 'scroll' @@ -29,6 +30,7 @@ const readonly = new Set([ 'innerHeight', 'outerWidth', 'outerHeight', + 'devicePixelRatio', 'online' ]); diff --git a/test/runtime/samples/window-binding-resize/_config.js b/test/runtime/samples/window-binding-resize/_config.js index c99e92a07b..d7f0282147 100644 --- a/test/runtime/samples/window-binding-resize/_config.js +++ b/test/runtime/samples/window-binding-resize/_config.js @@ -1,5 +1,5 @@ export default { - html: '
1024x768
', + html: '
1024x768
1
', before_test() { Object.defineProperties(window, { @@ -10,6 +10,10 @@ export default { innerHeight: { value: 768, configurable: true + }, + devicePixelRatio: { + value: 1, + configurable: true } }); }, @@ -27,13 +31,17 @@ export default { innerHeight: { value: 456, configurable: true + }, + devicePixelRatio: { + value: 2, + configurable: true } }); await window.dispatchEvent(event); assert.htmlEqual(target.innerHTML, ` -
567x456
+
567x456
2
`); } }; diff --git a/test/runtime/samples/window-binding-resize/main.svelte b/test/runtime/samples/window-binding-resize/main.svelte index 405f4e6e23..8ece184416 100644 --- a/test/runtime/samples/window-binding-resize/main.svelte +++ b/test/runtime/samples/window-binding-resize/main.svelte @@ -1,8 +1,10 @@ - + -
{width}x{height}
\ No newline at end of file +
{width}x{height}
+
{devicePixelRatio}
diff --git a/test/validator/samples/window-binding-invalid/errors.json b/test/validator/samples/window-binding-invalid/errors.json index 1277984258..04ecbaafcd 100644 --- a/test/validator/samples/window-binding-invalid/errors.json +++ b/test/validator/samples/window-binding-invalid/errors.json @@ -1,6 +1,6 @@ [{ "code": "invalid-binding", - "message": "'potato' is not a valid binding on — valid bindings are innerWidth, innerHeight, outerWidth, outerHeight, scrollX, scrollY or online", + "message": "'potato' is not a valid binding on — valid bindings are innerWidth, innerHeight, outerWidth, outerHeight, scrollX, scrollY, devicePixelRatio or online", "start": { "line": 1, "column": 15 From a74caf1381f4007a33bdeac43c3562c366c1a3a8 Mon Sep 17 00:00:00 2001 From: abirtley Date: Fri, 28 Apr 2023 00:34:23 +1000 Subject: [PATCH 16/20] docs: Clarify when bind:group does not work (#8540) Clarify documentation around when bind:group does and does not work. See issue #2308 --- site/content/docs/03-template-syntax.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/site/content/docs/03-template-syntax.md b/site/content/docs/03-template-syntax.md index 27d3ca9987..fe06b0c0bf 100644 --- a/site/content/docs/03-template-syntax.md +++ b/site/content/docs/03-template-syntax.md @@ -823,6 +823,8 @@ Inputs that work together can use `bind:group`. ``` +> `bind:group` only works if the inputs are in the same Svelte component. + #### bind:this ```sv From c4261abfde96f10576b860ccab82951bf2cc35ef Mon Sep 17 00:00:00 2001 From: Cas <6506529+ThaUnknown@users.noreply.github.com> Date: Fri, 28 Apr 2023 10:15:58 +0200 Subject: [PATCH 17/20] feat: document fullscreenElement and visibilityState bindings (#8507) --- elements/index.d.ts | 7 +- site/content/docs/03-template-syntax.md | 12 ++++ src/compiler/compile/nodes/Binding.ts | 3 +- src/compiler/compile/nodes/Document.ts | 21 ++++++ .../compile/render_dom/wrappers/Document.ts | 71 ++++++++++++++++++- .../document-binding-fullscreen/_config.js | 31 ++++++++ .../document-binding-fullscreen/main.svelte | 7 ++ 7 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 test/runtime/samples/document-binding-fullscreen/_config.js create mode 100644 test/runtime/samples/document-binding-fullscreen/main.svelte diff --git a/elements/index.d.ts b/elements/index.d.ts index dd91d14654..e7ed8901af 100644 --- a/elements/index.d.ts +++ b/elements/index.d.ts @@ -1078,6 +1078,11 @@ export interface SvelteMediaTimeRange { end: number; } +export interface SvelteDocumentAttributes extends HTMLAttributes { + readonly 'bind:fullscreenElement'?: Document['fullscreenElement'] | undefined | null; + readonly 'bind:visibilityState'?: Document['visibilityState'] | undefined | null; +} + export interface SvelteWindowAttributes extends HTMLAttributes { readonly 'bind:innerWidth'?: Window['innerWidth'] | undefined | null; readonly 'bind:innerHeight'?: Window['innerHeight'] | undefined | null; @@ -1592,7 +1597,7 @@ export interface SvelteHTMLElements { // Svelte specific 'svelte:window': SvelteWindowAttributes; - 'svelte:document': HTMLAttributes; + 'svelte:document': SvelteDocumentAttributes; 'svelte:body': HTMLAttributes; 'svelte:fragment': { slot?: string }; 'svelte:options': { [name: string]: any }; diff --git a/site/content/docs/03-template-syntax.md b/site/content/docs/03-template-syntax.md index fe06b0c0bf..8038e972be 100644 --- a/site/content/docs/03-template-syntax.md +++ b/site/content/docs/03-template-syntax.md @@ -1759,6 +1759,9 @@ All except `scrollX` and `scrollY` are readonly. ```sv ``` +```sv + +``` --- @@ -1773,6 +1776,15 @@ As with ``, this element may only appear the top level of your co /> ``` +--- + +You can also bind to the following properties: + +* `fullscreenElement` +* `visibilityState` + +All are readonly. + ### `` ```sv diff --git a/src/compiler/compile/nodes/Binding.ts b/src/compiler/compile/nodes/Binding.ts index 303506222f..f655554c81 100644 --- a/src/compiler/compile/nodes/Binding.ts +++ b/src/compiler/compile/nodes/Binding.ts @@ -9,6 +9,7 @@ import { TemplateNode } from '../../interfaces'; import Element from './Element'; import InlineComponent from './InlineComponent'; import Window from './Window'; +import Document from './Document'; import { clone } from '../../utils/clone'; import compiler_errors from '../compiler_errors'; import compiler_warnings from '../compiler_warnings'; @@ -36,7 +37,7 @@ export default class Binding extends Node { is_contextual: boolean; is_readonly: boolean; - constructor(component: Component, parent: Element | InlineComponent | Window, scope: TemplateScope, info: TemplateNode) { + constructor(component: Component, parent: Element | InlineComponent | Window | Document, scope: TemplateScope, info: TemplateNode) { super(component, parent, scope, info); if (info.expression.type !== 'Identifier' && info.expression.type !== 'MemberExpression') { diff --git a/src/compiler/compile/nodes/Document.ts b/src/compiler/compile/nodes/Document.ts index 653ccb627b..60264aa40e 100644 --- a/src/compiler/compile/nodes/Document.ts +++ b/src/compiler/compile/nodes/Document.ts @@ -1,14 +1,24 @@ import Node from './shared/Node'; +import Binding from './Binding'; import EventHandler from './EventHandler'; +import fuzzymatch from '../../utils/fuzzymatch'; import Action from './Action'; import Component from '../Component'; +import list from '../../utils/list'; import TemplateScope from './shared/TemplateScope'; import { Element } from '../../interfaces'; import compiler_warnings from '../compiler_warnings'; +import compiler_errors from '../compiler_errors'; + +const valid_bindings = [ + 'fullscreenElement', + 'visibilityState' +]; export default class Document extends Node { type: 'Document'; handlers: EventHandler[] = []; + bindings: Binding[] = []; actions: Action[] = []; constructor(component: Component, parent: Node, scope: TemplateScope, info: Element) { @@ -17,6 +27,17 @@ export default class Document extends Node { info.attributes.forEach((node) => { if (node.type === 'EventHandler') { this.handlers.push(new EventHandler(component, this, scope, node)); + } else if (node.type === 'Binding') { + if (!~valid_bindings.indexOf(node.name)) { + const match = fuzzymatch(node.name, valid_bindings); + if (match) { + return component.error(node, compiler_errors.invalid_binding_on(node.name, '', ` (did you mean '${match}'?)`)); + } else { + return component.error(node, compiler_errors.invalid_binding_on(node.name, '', ` — valid bindings are ${list(valid_bindings)}`)); + } + } + + this.bindings.push(new Binding(component, this, scope, node)); } else if (node.type === 'Action') { this.actions.push(new Action(component, this, scope, node)); } else { diff --git a/src/compiler/compile/render_dom/wrappers/Document.ts b/src/compiler/compile/render_dom/wrappers/Document.ts index 4f7c86c54f..0a9565e64c 100644 --- a/src/compiler/compile/render_dom/wrappers/Document.ts +++ b/src/compiler/compile/render_dom/wrappers/Document.ts @@ -1,6 +1,6 @@ import Block from '../Block'; import Wrapper from './shared/Wrapper'; -import { x } from 'code-red'; +import { b, x } from 'code-red'; import Document from '../../nodes/Document'; import { Identifier } from 'estree'; import EventHandler from './Element/EventHandler'; @@ -9,6 +9,16 @@ import { TemplateNode } from '../../../interfaces'; import Renderer from '../Renderer'; import add_actions from './shared/add_actions'; +const associated_events = { + fullscreenElement: ['fullscreenchange'], + visibilityState: ['visibilitychange'] +}; + +const readonly = new Set([ + 'fullscreenElement', + 'visibilityState' +]); + export default class DocumentWrapper extends Wrapper { node: Document; handlers: EventHandler[]; @@ -19,7 +29,66 @@ export default class DocumentWrapper extends Wrapper { } render(block: Block, _parent_node: Identifier, _parent_nodes: Identifier) { + const { renderer } = this; + const { component } = renderer; + + const events: Record> = {}; + const bindings: Record = {}; + add_event_handlers(block, x`@_document`, this.handlers); add_actions(block, x`@_document`, this.node.actions); + + this.node.bindings.forEach(binding => { + // TODO: what if it's a MemberExpression? + const binding_name = (binding.expression.node as Identifier).name; + + // in dev mode, throw if read-only values are written to + if (readonly.has(binding.name)) { + renderer.readonly.add(binding_name); + } + + bindings[binding.name] = binding_name; + + const binding_events = associated_events[binding.name]; + const property = binding.name; + + binding_events.forEach(associated_event => { + if (!events[associated_event]) events[associated_event] = []; + events[associated_event].push({ + name: binding_name, + value: property + }); + }); + }); + + Object.keys(events).forEach(event => { + const id = block.get_unique_name(`ondocument${event}`); + const props = events[event]; + + renderer.add_to_context(id.name); + const fn = renderer.reference(id.name); + + props.forEach(prop => { + renderer.meta_bindings.push( + b`this._state.${prop.name} = @_document.${prop.value};` + ); + }); + + block.event_listeners.push(x` + @listen(@_document, "${event}", ${fn}) + `); + + component.partly_hoisted.push(b` + function ${id}() { + ${props.map(prop => renderer.invalidate(prop.name, x`${prop.name} = @_document.${prop.value}`))} + } + `); + + block.chunks.init.push(b` + @add_render_callback(${fn}); + `); + + component.has_reactive_assignments = true; + }); } } diff --git a/test/runtime/samples/document-binding-fullscreen/_config.js b/test/runtime/samples/document-binding-fullscreen/_config.js new file mode 100644 index 0000000000..154ec0445a --- /dev/null +++ b/test/runtime/samples/document-binding-fullscreen/_config.js @@ -0,0 +1,31 @@ +export default { + before_test() { + Object.defineProperties(window.document, { + fullscreenElement: { + value: null, + configurable: true + } + }); + }, + + // copied from window-binding + // there's some kind of weird bug with this test... it compiles with the wrong require.extensions hook for some bizarre reason + skip_if_ssr: true, + + async test({ assert, target, window, component }) { + const event = new window.Event('fullscreenchange'); + + const div = target.querySelector('div'); + + Object.defineProperties(window.document, { + fullscreenElement: { + value: div, + configurable: true + } + }); + + window.document.dispatchEvent(event); + + assert.equal(component.fullscreen, div); + } +}; diff --git a/test/runtime/samples/document-binding-fullscreen/main.svelte b/test/runtime/samples/document-binding-fullscreen/main.svelte new file mode 100644 index 0000000000..5b00199821 --- /dev/null +++ b/test/runtime/samples/document-binding-fullscreen/main.svelte @@ -0,0 +1,7 @@ + + + + +
\ No newline at end of file From e45a1e05a341e95d30ddb097bc144d8d6f5b3573 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Tue, 2 May 2023 12:47:03 +0200 Subject: [PATCH 18/20] note of restructuring --- .github/PULL_REQUEST_TEMPLATE.md | 4 ++++ CONTRIBUTING.md | 2 ++ 2 files changed, 6 insertions(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 05b48f2ae0..28234b69df 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,3 +1,7 @@ +# HEADS UP: BIG RESTRUCTURING UNDERWAY + +The Svelte repo is currently in the process of heavy restructuring for Svelte 4. After that, work on Svelte 5 will likely change a lot on the compiler aswell. For that reason, please don't open PRs that are large in scope, touch more than a couple of files etc. In other words, bug fixes are fine, but feature PRs will likely not be merged. + ### Before submitting the PR, please make sure you do the following - [ ] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [ ] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 01f8728cc0..943006267e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,6 +62,8 @@ When [opening a new issue](https://github.com/sveltejs/svelte/issues/new/choose) ## Pull requests +> HEADS UP: The Svelte repo is currently in the process of heavy restructuring for Svelte 4. After that, work on Svelte 5 will likely change a lot on the compiler aswell. For that reason, please don't open PRs that are large in scope, touch more than a couple of files etc. In other words, bug fixes are fine, but feature PRs will likely not be merged. + ### Proposing a change If you would like to request a new feature or enhancement but are not yet thinking about opening a pull request, you can also file an issue with [feature template](https://github.com/sveltejs/svelte/issues/new?template=feature_request.yml). From 17bf6db5419a312577687dcca69a13a2c4ad478f Mon Sep 17 00:00:00 2001 From: Nguyen Tran <88808276+ngtr6788@users.noreply.github.com> Date: Thu, 4 May 2023 07:19:10 -0400 Subject: [PATCH 19/20] fix: Array rest property fix (#8553) Fixes #8552 --- src/compiler/compile/Component.ts | 18 ++++++---- .../array-rest-is-array-or-object/_config.js | 12 +++++++ .../array-rest-is-array-or-object/main.svelte | 15 ++++++++ .../samples/destructured-props-4/A.svelte | 25 +++++++++++++ .../samples/destructured-props-4/_config.js | 9 +++++ .../samples/destructured-props-4/main.svelte | 7 ++++ .../samples/destructured-props-5/A.svelte | 23 ++++++++++++ .../samples/destructured-props-5/_config.js | 19 ++++++++++ .../samples/destructured-props-5/main.svelte | 36 +++++++++++++++++++ 9 files changed, 157 insertions(+), 7 deletions(-) create mode 100644 test/runtime/samples/array-rest-is-array-or-object/_config.js create mode 100644 test/runtime/samples/array-rest-is-array-or-object/main.svelte create mode 100644 test/runtime/samples/destructured-props-4/A.svelte create mode 100644 test/runtime/samples/destructured-props-4/_config.js create mode 100644 test/runtime/samples/destructured-props-4/main.svelte create mode 100644 test/runtime/samples/destructured-props-5/A.svelte create mode 100644 test/runtime/samples/destructured-props-5/_config.js create mode 100644 test/runtime/samples/destructured-props-5/main.svelte diff --git a/src/compiler/compile/Component.ts b/src/compiler/compile/Component.ts index e87cf6218a..a756f8e3c4 100644 --- a/src/compiler/compile/Component.ts +++ b/src/compiler/compile/Component.ts @@ -25,7 +25,7 @@ import TemplateScope from './nodes/shared/TemplateScope'; import fuzzymatch from '../utils/fuzzymatch'; import get_object from './utils/get_object'; import Slot from './nodes/Slot'; -import { Node, ImportDeclaration, ExportNamedDeclaration, Identifier, ExpressionStatement, AssignmentExpression, Literal, Property, RestElement, ExportDefaultDeclaration, ExportAllDeclaration, FunctionDeclaration, FunctionExpression } from 'estree'; +import { Node, ImportDeclaration, ExportNamedDeclaration, Identifier, ExpressionStatement, AssignmentExpression, Literal, Property, RestElement, ExportDefaultDeclaration, ExportAllDeclaration, FunctionDeclaration, FunctionExpression, Pattern, Expression } from 'estree'; import add_to_set from './utils/add_to_set'; import check_graph_for_cycles from './utils/check_graph_for_cycles'; import { print, b } from 'code-red'; @@ -1034,7 +1034,7 @@ export default class Component { const inserts = []; const props = []; - function add_new_props(exported, local, default_value) { + function add_new_props(exported: Identifier, local: Pattern, default_value: Expression) { props.push({ type: 'Property', method: false, @@ -1064,7 +1064,7 @@ export default class Component { for (let index = 0; index < node.declarations.length; index++) { const declarator = node.declarations[index]; if (declarator.id.type !== 'Identifier') { - function get_new_name(local) { + function get_new_name(local: Identifier): Identifier { const variable = component.var_lookup.get(local.name); if (variable.subscribable) { inserts.push(get_insert(variable)); @@ -1078,7 +1078,7 @@ export default class Component { return local; } - function rename_identifiers(param: Node) { + function rename_identifiers(param: Pattern) { switch (param.type) { case 'ObjectPattern': { const handle_prop = (prop: Property | RestElement) => { @@ -1087,7 +1087,7 @@ export default class Component { } else if (prop.value.type === 'Identifier') { prop.value = get_new_name(prop.value); } else { - rename_identifiers(prop.value); + rename_identifiers(prop.value as Pattern); } }; @@ -1095,7 +1095,7 @@ export default class Component { break; } case 'ArrayPattern': { - const handle_element = (element: Node, index: number, array: Node[]) => { + const handle_element = (element: Pattern | null, index: number, array: Array) => { if (element) { if (element.type === 'Identifier') { array[index] = get_new_name(element); @@ -1110,7 +1110,11 @@ export default class Component { } case 'RestElement': - param.argument = get_new_name(param.argument); + if (param.argument.type === 'Identifier') { + param.argument = get_new_name(param.argument); + } else { + rename_identifiers(param.argument); + } break; case 'AssignmentPattern': diff --git a/test/runtime/samples/array-rest-is-array-or-object/_config.js b/test/runtime/samples/array-rest-is-array-or-object/_config.js new file mode 100644 index 0000000000..c971e109ce --- /dev/null +++ b/test/runtime/samples/array-rest-is-array-or-object/_config.js @@ -0,0 +1,12 @@ +export default { + html: ` +

1

+

2

+

3

+

5

+

10

+

20

+

30

+

6

+ ` +}; diff --git a/test/runtime/samples/array-rest-is-array-or-object/main.svelte b/test/runtime/samples/array-rest-is-array-or-object/main.svelte new file mode 100644 index 0000000000..fb3a5b7d85 --- /dev/null +++ b/test/runtime/samples/array-rest-is-array-or-object/main.svelte @@ -0,0 +1,15 @@ + + +

{first}

+

{second}

+

{third}

+

{fifth}

+ +

{one}

+

{two}

+

{three}

+

{length}

+ diff --git a/test/runtime/samples/destructured-props-4/A.svelte b/test/runtime/samples/destructured-props-4/A.svelte new file mode 100644 index 0000000000..ab5e6b7689 --- /dev/null +++ b/test/runtime/samples/destructured-props-4/A.svelte @@ -0,0 +1,25 @@ + + +
+a: {a}, +b: {typeof b}, +c: {c}, +d_one: {d_one}, +d_three: {$d_three}, +length: {length}, +f: {f}, +g: {g}, +e: {typeof e}, +e_one: {e_one}, +A: {A}, +C: {C} +
+
{JSON.stringify(THING)}
diff --git a/test/runtime/samples/destructured-props-4/_config.js b/test/runtime/samples/destructured-props-4/_config.js new file mode 100644 index 0000000000..2c48ac5b3a --- /dev/null +++ b/test/runtime/samples/destructured-props-4/_config.js @@ -0,0 +1,9 @@ +export default { + html: ` +
a: 1, b: undefined, c: 2, d_one: 3, d_three: 5, length: 2, f: undefined, g: 9, e: undefined, e_one: 6, A: 1, C: 2
+
{"a":1,"b":{"c":2,"d":[3,4,{},6,7]},"e":[6],"h":8}
+
+
a: a, b: undefined, c: 2, d_one: d_one, d_three: 5, length: 7, f: f, g: g, e: undefined, e_one: 6, A: 1, C: 2
+
{"a":1,"b":{"c":2,"d":[3,4,{},6,7]},"e":[6],"h":8}
+ ` +}; diff --git a/test/runtime/samples/destructured-props-4/main.svelte b/test/runtime/samples/destructured-props-4/main.svelte new file mode 100644 index 0000000000..cc1a31f542 --- /dev/null +++ b/test/runtime/samples/destructured-props-4/main.svelte @@ -0,0 +1,7 @@ + + + +
+
diff --git a/test/runtime/samples/destructured-props-5/A.svelte b/test/runtime/samples/destructured-props-5/A.svelte new file mode 100644 index 0000000000..898ce5aa3d --- /dev/null +++ b/test/runtime/samples/destructured-props-5/A.svelte @@ -0,0 +1,23 @@ + + +
+ x: {x}, list_two_a: {list_two_a}, list_two_b: {list_two_b}, y: {y}, l: {l}, m: {m}, + n: {n}, o: {o}, p: {p}, q: {$q}, r: {$r}, s: {s} +
+
{JSON.stringify(LIST)}
diff --git a/test/runtime/samples/destructured-props-5/_config.js b/test/runtime/samples/destructured-props-5/_config.js new file mode 100644 index 0000000000..6c8ca89216 --- /dev/null +++ b/test/runtime/samples/destructured-props-5/_config.js @@ -0,0 +1,19 @@ +export default { + html: ` +
x: 1, list_two_a: 4, list_two_b: 5, y: 3, l: 1, m: 2, n: 4, o: 5, p: 5, q: 6, r: 7, s: 1
+
[1,2,3,{"a":4},[5,{},{},8]]
+
x: 1, list_two_a: 4, list_two_b: 5, y: 3, l: l, m: m, n: n, o: o, p: p, q: q, r: r, s: s
+
[1,2,3,{"a":4},[5,{},{},8]]
+ `, + + async test({ component, assert, target }) { + await component.update(); + + assert.htmlEqual(target.innerHTML, ` +
x: 1, list_two_a: 4, list_two_b: 5, y: 3, l: 1, m: 2, n: 4, o: 5, p: 5, q: 6, r: 7, s: 1
+
[1,2,3,{"a":4},[5,{},{},8]]
+
x: 1, list_two_a: 4, list_two_b: 5, y: 3, l: LL, m: MM, n: NN, o: OO, p: PP, q: QQ, r: RR, s: SS
+
[1,2,3,{"a":4},[5,{},{},8]]
+ `); + } +}; diff --git a/test/runtime/samples/destructured-props-5/main.svelte b/test/runtime/samples/destructured-props-5/main.svelte new file mode 100644 index 0000000000..a8b9e8a704 --- /dev/null +++ b/test/runtime/samples/destructured-props-5/main.svelte @@ -0,0 +1,36 @@ + + +
+
+
From 83679e97001bf5d0d74f85b62cb69d083ae01d34 Mon Sep 17 00:00:00 2001 From: Nguyen Tran <88808276+ngtr6788@users.noreply.github.com> Date: Thu, 4 May 2023 07:59:33 -0400 Subject: [PATCH 20/20] fix: array rest destructuring in markup (#8555) Fixes #8554 --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> --- src/compiler/compile/compiler_warnings.ts | 2 +- src/compiler/compile/nodes/shared/Context.ts | 50 +++++++------- .../_config.js | 69 +++++++++++++++++++ .../main.svelte | 18 +++++ .../_config.js | 19 +++++ .../main.svelte | 23 +++++++ .../_config.js | 30 ++++++++ .../main.svelte | 19 +++++ .../_config.js | 24 +++++++ .../main.svelte | 9 +++ .../rest-eachblock-binding-2/warnings.json | 4 +- .../rest-eachblock-binding-3/warnings.json | 6 +- .../input.svelte | 9 +++ .../warnings.json | 26 +++++++ .../rest-eachblock-binding/warnings.json | 4 +- 15 files changed, 281 insertions(+), 31 deletions(-) create mode 100644 test/runtime/samples/await-then-destruct-array-nested-rest/_config.js create mode 100644 test/runtime/samples/await-then-destruct-array-nested-rest/main.svelte create mode 100644 test/runtime/samples/const-tag-await-then-destructuring-nested-rest/_config.js create mode 100644 test/runtime/samples/const-tag-await-then-destructuring-nested-rest/main.svelte create mode 100644 test/runtime/samples/const-tag-each-destructure-nested-rest/_config.js create mode 100644 test/runtime/samples/const-tag-each-destructure-nested-rest/main.svelte create mode 100644 test/runtime/samples/each-block-destructured-array-nested-rest/_config.js create mode 100644 test/runtime/samples/each-block-destructured-array-nested-rest/main.svelte create mode 100644 test/validator/samples/rest-eachblock-binding-nested-rest/input.svelte create mode 100644 test/validator/samples/rest-eachblock-binding-nested-rest/warnings.json diff --git a/src/compiler/compile/compiler_warnings.ts b/src/compiler/compile/compiler_warnings.ts index a851bc24c2..d778ca1dbc 100644 --- a/src/compiler/compile/compiler_warnings.ts +++ b/src/compiler/compile/compiler_warnings.ts @@ -220,7 +220,7 @@ export default { }, invalid_rest_eachblock_binding: (rest_element_name: string) => ({ code: 'invalid-rest-eachblock-binding', - message: `...${rest_element_name} operator will create a new object and binding propagation with original object will not work` + message: `The rest operator (...) will create a new object and binding '${rest_element_name}' with the original object will not work` }), avoid_mouse_events_on_document: { code: 'avoid-mouse-events-on-document', diff --git a/src/compiler/compile/nodes/shared/Context.ts b/src/compiler/compile/nodes/shared/Context.ts index 76ac895681..43d4d1f626 100644 --- a/src/compiler/compile/nodes/shared/Context.ts +++ b/src/compiler/compile/nodes/shared/Context.ts @@ -1,5 +1,5 @@ import { x } from 'code-red'; -import { Node, Identifier, Expression, PrivateIdentifier } from 'estree'; +import { Node, Identifier, Expression, PrivateIdentifier, Pattern } from 'estree'; import { walk } from 'estree-walker'; import is_reference, { NodeWithPropertyDefinition } from 'is-reference'; import { clone } from '../../../utils/clone'; @@ -30,15 +30,17 @@ export function unpack_destructuring({ default_modifier = (node) => node, scope, component, - context_rest_properties + context_rest_properties, + in_rest_element = false }: { contexts: Context[]; - node: Node; + node: Pattern; modifier?: DestructuredVariable['modifier']; default_modifier?: DestructuredVariable['default_modifier']; scope: TemplateScope; component: Component; context_rest_properties: Map; + in_rest_element?: boolean; }) { if (!node) return; @@ -49,28 +51,26 @@ export function unpack_destructuring({ modifier, default_modifier }); - } else if (node.type === 'RestElement') { - contexts.push({ - type: 'DestructuredVariable', - key: node.argument as Identifier, - modifier, - default_modifier - }); - context_rest_properties.set((node.argument as Identifier).name, node); + + if (in_rest_element) { + context_rest_properties.set(node.name, node); + } } else if (node.type === 'ArrayPattern') { - node.elements.forEach((element, i) => { - if (element && element.type === 'RestElement') { + node.elements.forEach((element: Pattern | null, i: number) => { + if (!element) { + return; + } else if (element.type === 'RestElement') { unpack_destructuring({ contexts, - node: element, + node: element.argument, modifier: (node) => x`${modifier(node)}.slice(${i})` as Node, default_modifier, scope, component, - context_rest_properties + context_rest_properties, + in_rest_element: true }); - context_rest_properties.set((element.argument as Identifier).name, element); - } else if (element && element.type === 'AssignmentPattern') { + } else if (element.type === 'AssignmentPattern') { const n = contexts.length; mark_referenced(element.right, scope, component); @@ -87,7 +87,8 @@ export function unpack_destructuring({ )}` as Node, scope, component, - context_rest_properties + context_rest_properties, + in_rest_element }); } else { unpack_destructuring({ @@ -97,7 +98,8 @@ export function unpack_destructuring({ default_modifier, scope, component, - context_rest_properties + context_rest_properties, + in_rest_element }); } }); @@ -116,9 +118,9 @@ export function unpack_destructuring({ default_modifier, scope, component, - context_rest_properties + context_rest_properties, + in_rest_element: true }); - context_rest_properties.set((property.argument as Identifier).name, property); } else if (property.type === 'Property') { const key = property.key; const value = property.value; @@ -168,7 +170,8 @@ export function unpack_destructuring({ )}` as Node, scope, component, - context_rest_properties + context_rest_properties, + in_rest_element }); } else { // e.g. { property } or { property: newName } @@ -179,7 +182,8 @@ export function unpack_destructuring({ default_modifier, scope, component, - context_rest_properties + context_rest_properties, + in_rest_element }); } } diff --git a/test/runtime/samples/await-then-destruct-array-nested-rest/_config.js b/test/runtime/samples/await-then-destruct-array-nested-rest/_config.js new file mode 100644 index 0000000000..9a287e35e2 --- /dev/null +++ b/test/runtime/samples/await-then-destruct-array-nested-rest/_config.js @@ -0,0 +1,69 @@ +export default { + props: { + thePromise: new Promise(_ => {}) + }, + + html: ` + loading... + `, + + async test({ assert, component, target }) { + await (component.thePromise = Promise.resolve([1, 2, 3, 4, 5, 6, 7, 8])); + + assert.htmlEqual( + target.innerHTML, + ` +

a: 1

+

b: 2

+

c: 5

+

remaining length: 3

+ ` + ); + + await (component.thePromise = Promise.resolve([9, 10, 11, 12, 13, 14, 15])); + + assert.htmlEqual( + target.innerHTML, + ` +

a: 9

+

b: 10

+

c: 13

+

remaining length: 2

+ ` + ); + + try { + await (component.thePromise = Promise.reject([16, 17, 18, 19, 20, 21, 22])); + } catch (e) { + // do nothing + } + + assert.htmlEqual( + target.innerHTML, + ` +

c: 16

+

d: 17

+

e: 18

+

f: 19

+

g: 22

+ ` + ); + + try { + await (component.thePromise = Promise.reject([23, 24, 25, 26, 27, 28, 29, 30, 31])); + } catch (e) { + // do nothing + } + + assert.htmlEqual( + target.innerHTML, + ` +

c: 23

+

d: 24

+

e: 25

+

f: 26

+

g: 29

+ ` + ); + } +}; diff --git a/test/runtime/samples/await-then-destruct-array-nested-rest/main.svelte b/test/runtime/samples/await-then-destruct-array-nested-rest/main.svelte new file mode 100644 index 0000000000..4bb8ad0077 --- /dev/null +++ b/test/runtime/samples/await-then-destruct-array-nested-rest/main.svelte @@ -0,0 +1,18 @@ + + +{#await thePromise} + loading... +{:then [ a, b, ...[,, c, ...{ length } ]]} +

a: {a}

+

b: {b}

+

c: {c}

+

remaining length: {length}

+{:catch [c, ...[d, e, f, ...[,,g]]]} +

c: {c}

+

d: {d}

+

e: {e}

+

f: {f}

+

g: {g}

+{/await} diff --git a/test/runtime/samples/const-tag-await-then-destructuring-nested-rest/_config.js b/test/runtime/samples/const-tag-await-then-destructuring-nested-rest/_config.js new file mode 100644 index 0000000000..cb40e7c456 --- /dev/null +++ b/test/runtime/samples/const-tag-await-then-destructuring-nested-rest/_config.js @@ -0,0 +1,19 @@ +export default { + html: '
12 120 70, 30+4=34
', + async test({ component, target, assert }) { + component.promise1 = Promise.resolve({width: 5, height: 6}); + component.promise2 = Promise.reject({width: 6, height: 7}); + + await Promise.resolve(); + assert.htmlEqual(target.innerHTML, ` +
30 300 110, 50+6=56
+
42 420 130, 60+7=67
+ `); + + component.constant = 20; + assert.htmlEqual(target.innerHTML, ` +
30 600 220, 100+6=106
+
42 840 260, 120+7=127
+ `); + } +}; diff --git a/test/runtime/samples/const-tag-await-then-destructuring-nested-rest/main.svelte b/test/runtime/samples/const-tag-await-then-destructuring-nested-rest/main.svelte new file mode 100644 index 0000000000..7af5c989e4 --- /dev/null +++ b/test/runtime/samples/const-tag-await-then-destructuring-nested-rest/main.svelte @@ -0,0 +1,23 @@ + + +{#await promise1 then { width, height }} + {@const {area, volume} = calculate(width, height, constant)} + {@const perimeter = (width + height) * constant} + {@const [_width, ...[_height, ...[sum]]] = [width * constant, height, width * constant + height]} +
{area} {volume} {perimeter}, {_width}+{_height}={sum}
+{/await} + +{#await promise2 catch { width, height }} + {@const {area, volume} = calculate(width, height, constant)} + {@const perimeter = (width + height) * constant} + {@const [_width, ...[_height, ...[sum]]] = [width * constant, height, width * constant + height]} +
{area} {volume} {perimeter}, {_width}+{_height}={sum}
+{/await} diff --git a/test/runtime/samples/const-tag-each-destructure-nested-rest/_config.js b/test/runtime/samples/const-tag-each-destructure-nested-rest/_config.js new file mode 100644 index 0000000000..00f8b31540 --- /dev/null +++ b/test/runtime/samples/const-tag-each-destructure-nested-rest/_config.js @@ -0,0 +1,30 @@ +export default { + html: ` +
12 120 70, 30+4=34
+
35 350 120, 50+7=57
+
48 480 140, 60+8=68
+ `, + async test({ component, target, assert }) { + component.constant = 20; + + assert.htmlEqual(target.innerHTML, ` +
12 240 140, 60+4=64
+
35 700 240, 100+7=107
+
48 960 280, 120+8=128
+ `); + + component.boxes = [ + {width: 3, height: 4}, + {width: 4, height: 5}, + {width: 5, height: 6}, + {width: 6, height: 7} + ]; + + assert.htmlEqual(target.innerHTML, ` +
12 240 140, 60+4=64
+
20 400 180, 80+5=85
+
30 600 220, 100+6=106
+
42 840 260, 120+7=127
+ `); + } +}; diff --git a/test/runtime/samples/const-tag-each-destructure-nested-rest/main.svelte b/test/runtime/samples/const-tag-each-destructure-nested-rest/main.svelte new file mode 100644 index 0000000000..4361314b19 --- /dev/null +++ b/test/runtime/samples/const-tag-each-destructure-nested-rest/main.svelte @@ -0,0 +1,19 @@ + + +{#each boxes as { width, height }} + {@const {area, volume} = calculate(width, height, constant)} + {@const perimeter = (width + height) * constant} + {@const [_width, ...[_height, ...[sum]]] = [width * constant, height, width * constant + height]} +
{area} {volume} {perimeter}, {_width}+{_height}={sum}
+{/each} diff --git a/test/runtime/samples/each-block-destructured-array-nested-rest/_config.js b/test/runtime/samples/each-block-destructured-array-nested-rest/_config.js new file mode 100644 index 0000000000..5922925956 --- /dev/null +++ b/test/runtime/samples/each-block-destructured-array-nested-rest/_config.js @@ -0,0 +1,24 @@ +export default { + props: { + array: [ + [1, 2, 3, 4, 5], + [6, 7, 8], + [9, 10, 11, 12], + [13, 14, 15, 16, 17, 18, 19, 20, 21, 22] + ] + }, + + html: ` +

First: 1, Second: 2, Third: 3, Elements remaining: 2

+

First: 6, Second: 7, Third: 8, Elements remaining: 0

+

First: 9, Second: 10, Third: 11, Elements remaining: 1

+

First: 13, Second: 14, Third: 15, Elements remaining: 7

+ `, + + test({ assert, component, target }) { + component.array = [[23, 24, 25, 26, 27, 28, 29]]; + assert.htmlEqual( target.innerHTML, ` +

First: 23, Second: 24, Third: 25, Elements remaining: 4

+ `); + } +}; diff --git a/test/runtime/samples/each-block-destructured-array-nested-rest/main.svelte b/test/runtime/samples/each-block-destructured-array-nested-rest/main.svelte new file mode 100644 index 0000000000..fc73462464 --- /dev/null +++ b/test/runtime/samples/each-block-destructured-array-nested-rest/main.svelte @@ -0,0 +1,9 @@ + + +{#each array as [first, second, ...[third, ...{ length }]]} +

+ First: {first}, Second: {second}, Third: {third}, Elements remaining: {length} +

+{/each} diff --git a/test/validator/samples/rest-eachblock-binding-2/warnings.json b/test/validator/samples/rest-eachblock-binding-2/warnings.json index a471dd6a86..174982f21b 100644 --- a/test/validator/samples/rest-eachblock-binding-2/warnings.json +++ b/test/validator/samples/rest-eachblock-binding-2/warnings.json @@ -1,8 +1,8 @@ [ { "code": "invalid-rest-eachblock-binding", - "message": "...rest operator will create a new object and binding propagation with original object will not work", - "start": { "line": 8, "column": 24 }, + "message": "The rest operator (...) will create a new object and binding 'rest' with the original object will not work", + "start": { "line": 8, "column": 27 }, "end": { "line": 8, "column": 31 } } ] diff --git a/test/validator/samples/rest-eachblock-binding-3/warnings.json b/test/validator/samples/rest-eachblock-binding-3/warnings.json index eda7e1fc5d..3311d2afe4 100644 --- a/test/validator/samples/rest-eachblock-binding-3/warnings.json +++ b/test/validator/samples/rest-eachblock-binding-3/warnings.json @@ -1,8 +1,8 @@ [ { "code": "invalid-rest-eachblock-binding", - "message": "...rest operator will create a new object and binding propagation with original object will not work", - "start": { "line": 5, "column": 32 }, - "end": { "line": 5, "column": 39 } + "message": "The rest operator (...) will create a new object and binding 'rest' with the original object will not work", + "start": { "line": 5, "column": 35 }, + "end": { "line": 5, "column": 39 } } ] diff --git a/test/validator/samples/rest-eachblock-binding-nested-rest/input.svelte b/test/validator/samples/rest-eachblock-binding-nested-rest/input.svelte new file mode 100644 index 0000000000..31f32975f6 --- /dev/null +++ b/test/validator/samples/rest-eachblock-binding-nested-rest/input.svelte @@ -0,0 +1,9 @@ + + +{#each a as [first, second, ...[third, ...{ length }]]} +

{first}, {second}, {length}

+ + +{/each} diff --git a/test/validator/samples/rest-eachblock-binding-nested-rest/warnings.json b/test/validator/samples/rest-eachblock-binding-nested-rest/warnings.json new file mode 100644 index 0000000000..d935275edf --- /dev/null +++ b/test/validator/samples/rest-eachblock-binding-nested-rest/warnings.json @@ -0,0 +1,26 @@ +[ + { + "code": "invalid-rest-eachblock-binding", + "end": { + "column": 37, + "line": 5 + }, + "message": "The rest operator (...) will create a new object and binding 'third' with the original object will not work", + "start": { + "column": 32, + "line": 5 + } + }, + { + "code": "invalid-rest-eachblock-binding", + "end": { + "column": 50, + "line": 5 + }, + "message": "The rest operator (...) will create a new object and binding 'length' with the original object will not work", + "start": { + "column": 44, + "line": 5 + } + } +] diff --git a/test/validator/samples/rest-eachblock-binding/warnings.json b/test/validator/samples/rest-eachblock-binding/warnings.json index 35fb2d0b6e..992e8880d9 100644 --- a/test/validator/samples/rest-eachblock-binding/warnings.json +++ b/test/validator/samples/rest-eachblock-binding/warnings.json @@ -1,8 +1,8 @@ [ { "code": "invalid-rest-eachblock-binding", - "message": "...rest operator will create a new object and binding propagation with original object will not work", - "start": { "line": 5, "column": 25 }, + "message": "The rest operator (...) will create a new object and binding 'rest' with the original object will not work", + "start": { "line": 5, "column": 28 }, "end": { "line": 5, "column": 32 } } ]