diff --git a/CHANGELOG.md b/CHANGELOG.md index 4081405811..39dc47b387 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,13 @@ # Svelte changelog -## Unreleased +## 3.45.0 +* Fix non-boolean attribute rendering in SSR to render truthy values as-is ([#6121](https://github.com/sveltejs/svelte/issues/6121)) +* Fix binding to a member expression also invalidating the member property ([#6921](https://github.com/sveltejs/svelte/issues/6921)) +* Fix default values in `{#each}`/etc. destructurings not being considered references for the purposes of compiler warnings ([#6964](https://github.com/sveltejs/svelte/issues/6964)) +* Fix `{:else if}` value incorrectly being cached ([#7043](https://github.com/sveltejs/svelte/pull/7043)) * Add `a11y-no-redundant-roles` warning ([#7067](https://github.com/sveltejs/svelte/pull/7067)) +* Fix code generation error with arrow functions whose bodies are object destructuring assignments ([#7087](https://github.com/sveltejs/svelte/issues/7087)) ## 3.44.3 diff --git a/package-lock.json b/package-lock.json index 0aeed9166d..c0f623415d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "svelte", - "version": "3.44.3", + "version": "3.45.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "svelte", - "version": "3.44.3", + "version": "3.45.0", "license": "MIT", "devDependencies": { "@ampproject/remapping": "^0.3.0", @@ -25,7 +25,7 @@ "acorn": "^8.4.1", "agadoo": "^1.1.0", "c8": "^5.0.1", - "code-red": "^0.2.3", + "code-red": "^0.2.4", "codecov": "^3.5.0", "css-tree": "^1.1.2", "eslint": "^7.32.0", @@ -1082,9 +1082,9 @@ } }, "node_modules/code-red": { - "version": "0.2.3", - "resolved": "https://registry.npmjs.org/code-red/-/code-red-0.2.3.tgz", - "integrity": "sha512-l9MRiYO9iNx3dCpoZBZkaHAVtbhig8TBddEHq7ssWcZRAjaYR8NoRFzZ56VJ20TIg7hEQegCVCH3fVus+2Ol4Q==", + "version": "0.2.4", + "resolved": "https://registry.npmjs.org/code-red/-/code-red-0.2.4.tgz", + "integrity": "sha512-tAJQiZviSyB2KUhz+rocKFzCHPkVooX2aFrdpfWDRvxWJaBQTYFJ/Z2TcWqbjXj5oJJBlqd2GxBXdtAhOXySVQ==", "dev": true, "dependencies": { "@types/estree": "^0.0.50", @@ -6376,9 +6376,9 @@ } }, "code-red": { - "version": "0.2.3", - "resolved": "https://registry.npmjs.org/code-red/-/code-red-0.2.3.tgz", - "integrity": "sha512-l9MRiYO9iNx3dCpoZBZkaHAVtbhig8TBddEHq7ssWcZRAjaYR8NoRFzZ56VJ20TIg7hEQegCVCH3fVus+2Ol4Q==", + "version": "0.2.4", + "resolved": "https://registry.npmjs.org/code-red/-/code-red-0.2.4.tgz", + "integrity": "sha512-tAJQiZviSyB2KUhz+rocKFzCHPkVooX2aFrdpfWDRvxWJaBQTYFJ/Z2TcWqbjXj5oJJBlqd2GxBXdtAhOXySVQ==", "dev": true, "requires": { "@types/estree": "^0.0.50", diff --git a/package.json b/package.json index 233772b8a2..7d36c10971 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "svelte", - "version": "3.44.3", + "version": "3.45.0", "description": "Cybernetically enhanced web apps", "module": "index.mjs", "main": "index", @@ -132,7 +132,7 @@ "acorn": "^8.4.1", "agadoo": "^1.1.0", "c8": "^5.0.1", - "code-red": "^0.2.3", + "code-red": "^0.2.4", "codecov": "^3.5.0", "css-tree": "^1.1.2", "eslint": "^7.32.0", diff --git a/src/compiler/compile/nodes/AwaitBlock.ts b/src/compiler/compile/nodes/AwaitBlock.ts index ef74d26848..735fdbfff3 100644 --- a/src/compiler/compile/nodes/AwaitBlock.ts +++ b/src/compiler/compile/nodes/AwaitBlock.ts @@ -33,12 +33,12 @@ export default class AwaitBlock extends Node { if (this.then_node) { this.then_contexts = []; - unpack_destructuring(this.then_contexts, info.value); + unpack_destructuring({ contexts: this.then_contexts, node: info.value, scope, component }); } if (this.catch_node) { this.catch_contexts = []; - unpack_destructuring(this.catch_contexts, info.error); + unpack_destructuring({ contexts: this.catch_contexts, node: info.error, scope, component }); } this.pending = new PendingBlock(component, this, scope, info.pending); diff --git a/src/compiler/compile/nodes/EachBlock.ts b/src/compiler/compile/nodes/EachBlock.ts index c03128c388..a8312b2462 100644 --- a/src/compiler/compile/nodes/EachBlock.ts +++ b/src/compiler/compile/nodes/EachBlock.ts @@ -39,7 +39,7 @@ export default class EachBlock extends AbstractBlock { this.scope = scope.child(); this.contexts = []; - unpack_destructuring(this.contexts, info.context); + unpack_destructuring({ contexts: this.contexts, node: info.context, scope, component }); this.contexts.forEach(context => { this.scope.add(context.key.name, this.expression.dependencies, this); diff --git a/src/compiler/compile/nodes/shared/Context.ts b/src/compiler/compile/nodes/shared/Context.ts index 6cc5a4358e..7507d1e37e 100644 --- a/src/compiler/compile/nodes/shared/Context.ts +++ b/src/compiler/compile/nodes/shared/Context.ts @@ -3,6 +3,9 @@ import { Node, Identifier, Expression } from 'estree'; import { walk } from 'estree-walker'; import is_reference, { NodeWithPropertyDefinition } from 'is-reference'; import { clone } from '../../../utils/clone'; +import Component from '../../Component'; +import flatten_reference from '../../utils/flatten_reference'; +import TemplateScope from './TemplateScope'; export interface Context { key: Identifier; @@ -11,7 +14,21 @@ export interface Context { default_modifier: (node: Node, to_ctx: (name: string) => Node) => Node; } -export function unpack_destructuring(contexts: Context[], node: Node, modifier: Context['modifier'] = node => node, default_modifier: Context['default_modifier'] = node => node) { +export function unpack_destructuring({ + contexts, + node, + modifier = (node) => node, + default_modifier = (node) => node, + scope, + component +}: { + contexts: Context[]; + node: Node; + modifier?: Context['modifier']; + default_modifier?: Context['default_modifier']; + scope: TemplateScope; + component: Component; +}) { if (!node) return; if (node.type === 'Identifier') { @@ -29,13 +46,41 @@ export function unpack_destructuring(contexts: Context[], node: Node, modifier: } else if (node.type === 'ArrayPattern') { node.elements.forEach((element, i) => { if (element && element.type === 'RestElement') { - unpack_destructuring(contexts, element, node => x`${modifier(node)}.slice(${i})` as Node, default_modifier); + unpack_destructuring({ + contexts, + node: element, + modifier: (node) => x`${modifier(node)}.slice(${i})` as Node, + default_modifier, + scope, + component + }); } else if (element && element.type === 'AssignmentPattern') { const n = contexts.length; + mark_referenced(element.right, scope, component); - unpack_destructuring(contexts, element.left, node => x`${modifier(node)}[${i}]`, (node, to_ctx) => x`${node} !== undefined ? ${node} : ${update_reference(contexts, n, element.right, to_ctx)}` as Node); + unpack_destructuring({ + contexts, + node: element.left, + modifier: (node) => x`${modifier(node)}[${i}]`, + default_modifier: (node, to_ctx) => + x`${node} !== undefined ? ${node} : ${update_reference( + contexts, + n, + element.right, + to_ctx + )}` as Node, + scope, + component + }); } else { - unpack_destructuring(contexts, element, node => x`${modifier(node)}[${i}]` as Node, default_modifier); + unpack_destructuring({ + contexts, + node: element, + modifier: (node) => x`${modifier(node)}[${i}]` as Node, + default_modifier, + scope, + component + }); } }); } else if (node.type === 'ObjectPattern') { @@ -43,12 +88,17 @@ export function unpack_destructuring(contexts: Context[], node: Node, modifier: node.properties.forEach((property) => { if (property.type === 'RestElement') { - unpack_destructuring( + unpack_destructuring({ contexts, - property.argument, - node => x`@object_without_properties(${modifier(node)}, [${used_properties}])` as Node, - default_modifier - ); + node: property.argument, + modifier: (node) => + x`@object_without_properties(${modifier( + node + )}, [${used_properties}])` as Node, + default_modifier, + scope, + component + }); } else { const key = property.key as Identifier; const value = property.value; @@ -57,16 +107,43 @@ export function unpack_destructuring(contexts: Context[], node: Node, modifier: if (value.type === 'AssignmentPattern') { const n = contexts.length; - unpack_destructuring(contexts, value.left, node => x`${modifier(node)}.${key.name}`, (node, to_ctx) => x`${node} !== undefined ? ${node} : ${update_reference(contexts, n, value.right, to_ctx)}` as Node); + mark_referenced(value.right, scope, component); + + unpack_destructuring({ + contexts, + node: value.left, + modifier: (node) => x`${modifier(node)}.${key.name}`, + default_modifier: (node, to_ctx) => + x`${node} !== undefined ? ${node} : ${update_reference( + contexts, + n, + value.right, + to_ctx + )}` as Node, + scope, + component + }); } else { - unpack_destructuring(contexts, value, node => x`${modifier(node)}.${key.name}` as Node, default_modifier); + unpack_destructuring({ + contexts, + node: value, + modifier: (node) => x`${modifier(node)}.${key.name}` as Node, + default_modifier, + scope, + component + }); } } }); } } -function update_reference(contexts: Context[], n: number, expression: Expression, to_ctx: (name: string) => Node): Node { +function update_reference( + contexts: Context[], + n: number, + expression: Expression, + to_ctx: (name: string) => Node +): Node { const find_from_context = (node: Identifier) => { for (let i = n; i < contexts.length; i++) { const { key } = contexts[i]; @@ -85,7 +162,12 @@ function update_reference(contexts: Context[], n: number, expression: Expression expression = clone(expression) as Expression; walk(expression, { enter(node, parent: Node) { - if (is_reference(node as NodeWithPropertyDefinition, parent as NodeWithPropertyDefinition)) { + if ( + is_reference( + node as NodeWithPropertyDefinition, + parent as NodeWithPropertyDefinition + ) + ) { this.replace(find_from_context(node as Identifier)); this.skip(); } @@ -94,3 +176,20 @@ function update_reference(contexts: Context[], n: number, expression: Expression return expression; } + +function mark_referenced( + node: Node, + scope: TemplateScope, + component: Component +) { + walk(node, { + enter(node: any, parent: any) { + if (is_reference(node, parent)) { + const { name } = flatten_reference(node); + if (!scope.is_let(name) && !scope.names.has(name)) { + component.add_reference(name); + } + } + } + }); +} diff --git a/src/compiler/compile/render_dom/wrappers/Element/Binding.ts b/src/compiler/compile/render_dom/wrappers/Element/Binding.ts index 7c8a339d00..bc218a510b 100644 --- a/src/compiler/compile/render_dom/wrappers/Element/Binding.ts +++ b/src/compiler/compile/render_dom/wrappers/Element/Binding.ts @@ -70,6 +70,30 @@ export default class BindingWrapper { return dependencies; } + get_update_dependencies() { + const object = this.object; + const dependencies = new Set(); + if (this.node.expression.template_scope.names.has(object)) { + this.node.expression.template_scope.dependencies_for_name + .get(object) + .forEach((name) => dependencies.add(name)); + } else { + dependencies.add(object); + } + + const result = new Set(dependencies); + dependencies.forEach((dependency) => { + const indirect_dependencies = this.parent.renderer.component.indirect_dependencies.get(dependency); + if (indirect_dependencies) { + indirect_dependencies.forEach(indirect_dependency => { + result.add(indirect_dependency); + }); + } + }); + + return result; + } + is_readonly_media_attribute() { return this.node.is_readonly_media_attribute(); } diff --git a/src/compiler/compile/render_dom/wrappers/Element/index.ts b/src/compiler/compile/render_dom/wrappers/Element/index.ts index 9ec36b12d7..583274044b 100644 --- a/src/compiler/compile/render_dom/wrappers/Element/index.ts +++ b/src/compiler/compile/render_dom/wrappers/Element/index.ts @@ -466,7 +466,7 @@ export default class ElementWrapper extends Wrapper { binding_group.bindings.forEach(binding => { // TODO this is a mess - add_to_set(dependencies, binding.get_dependencies()); + add_to_set(dependencies, binding.get_update_dependencies()); add_to_set(contextual_dependencies, binding.handler.contextual_dependencies); binding.render(block, lock); diff --git a/src/compiler/compile/render_dom/wrappers/IfBlock.ts b/src/compiler/compile/render_dom/wrappers/IfBlock.ts index b3bbf9becd..01d49ac9dd 100644 --- a/src/compiler/compile/render_dom/wrappers/IfBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/IfBlock.ts @@ -266,15 +266,15 @@ export default class IfBlockWrapper extends Wrapper { if (this.needs_update) { block.chunks.init.push(b` function ${select_block_type}(#ctx, #dirty) { - ${this.branches.map(({ dependencies, condition, snippet, block }) => condition + ${this.branches.map(({ dependencies, condition, snippet }) => { + return b`${snippet && dependencies.length > 0 ? b`if (${block.renderer.dirty(dependencies)}) ${condition} = null;` : null}`; + })} + ${this.branches.map(({ condition, snippet, block }) => condition ? b` - ${snippet && ( - dependencies.length > 0 - ? b`if (${condition} == null || ${block.renderer.dirty(dependencies)}) ${condition} = !!${snippet}` - : b`if (${condition} == null) ${condition} = !!${snippet}` + ${snippet && b`if (${condition} == null) ${condition} = !!${snippet}`} + if (${condition}) return ${block.name};` + : b`return ${block.name};` )} - if (${condition}) return ${block.name};` - : b`return ${block.name};`)} } `); } else { @@ -387,13 +387,12 @@ export default class IfBlockWrapper extends Wrapper { ${this.needs_update ? b` function ${select_block_type}(#ctx, #dirty) { - ${this.branches.map(({ dependencies, condition, snippet }, i) => condition + ${this.branches.map(({ dependencies, condition, snippet }) => { + return b`${snippet && dependencies.length > 0 ? b`if (${block.renderer.dirty(dependencies)}) ${condition} = null;` : null}`; + })} + ${this.branches.map(({ condition, snippet }, i) => condition ? b` - ${snippet && ( - dependencies.length > 0 - ? b`if (${condition} == null || ${block.renderer.dirty(dependencies)}) ${condition} = !!${snippet}` - : b`if (${condition} == null) ${condition} = !!${snippet}` - )} + ${snippet && b`if (${condition} == null) ${condition} = !!${snippet}`} if (${condition}) return ${i};` : b`return ${i};`)} ${!has_else && b`return -1;`} diff --git a/src/compiler/compile/render_dom/wrappers/shared/bind_this.ts b/src/compiler/compile/render_dom/wrappers/shared/bind_this.ts index 1b8c4f9db8..af440b6341 100644 --- a/src/compiler/compile/render_dom/wrappers/shared/bind_this.ts +++ b/src/compiler/compile/render_dom/wrappers/shared/bind_this.ts @@ -12,7 +12,7 @@ export default function bind_this(component: Component, block: Block, binding: B const callee = block.renderer.reference(fn.name); const { contextual_dependencies, mutation } = binding.handler; - const dependencies = binding.get_dependencies(); + const dependencies = binding.get_update_dependencies(); const body = b` ${mutation} diff --git a/src/runtime/internal/ssr.ts b/src/runtime/internal/ssr.ts index fd72aa1548..4d91857ad4 100644 --- a/src/runtime/internal/ssr.ts +++ b/src/runtime/internal/ssr.ts @@ -141,7 +141,7 @@ export function create_ssr_component(fn) { export function add_attribute(name, value, boolean) { if (value == null || (boolean && !value)) return ''; - return ` ${name}${value === true ? '' : `=${typeof value === 'string' ? JSON.stringify(escape(value)) : `"${value}"`}`}`; + return ` ${name}${value === true && boolean_attributes.has(name) ? '' : `=${typeof value === 'string' ? JSON.stringify(escape(value)) : `"${value}"`}`}`; } export function add_classes(classes) { diff --git a/test/runtime/samples/attribute-boolean-true/_config.js b/test/runtime/samples/attribute-boolean-true/_config.js index 79c8ea1ad6..89b2fc7c21 100644 --- a/test/runtime/samples/attribute-boolean-true/_config.js +++ b/test/runtime/samples/attribute-boolean-true/_config.js @@ -1,7 +1,8 @@ export default { - html: '', + html: '', test({ assert, target }) { const textarea = target.querySelector('textarea'); + assert.equal(textarea.dataset.attr, 'true'); assert.ok(textarea.readOnly); } }; diff --git a/test/runtime/samples/attribute-boolean-true/main.svelte b/test/runtime/samples/attribute-boolean-true/main.svelte index 04d5cab8f9..6474644749 100644 --- a/test/runtime/samples/attribute-boolean-true/main.svelte +++ b/test/runtime/samples/attribute-boolean-true/main.svelte @@ -1 +1 @@ - \ No newline at end of file +