From d7c92d9e903252d4df14fe709b5f26431a4f0579 Mon Sep 17 00:00:00 2001 From: adiguba Date: Sat, 21 Dec 2024 11:20:54 +0100 Subject: [PATCH 1/6] first implementation for display directive --- .../compiler/phases/1-parse/state/element.js | 19 +++++ .../client/visitors/RegularElement.js | 59 +++++++++++++- .../server/visitors/shared/element.js | 50 ++++++++++-- .../svelte/src/compiler/types/template.d.ts | 12 ++- .../src/internal/client/dom/blocks/display.js | 80 +++++++++++++++++++ packages/svelte/src/internal/client/index.js | 1 + packages/svelte/types/index.d.ts | 8 +- 7 files changed, 220 insertions(+), 9 deletions(-) create mode 100644 packages/svelte/src/internal/client/dom/blocks/display.js diff --git a/packages/svelte/src/compiler/phases/1-parse/state/element.js b/packages/svelte/src/compiler/phases/1-parse/state/element.js index 66946a8f8d22..d01f9296ce67 100644 --- a/packages/svelte/src/compiler/phases/1-parse/state/element.js +++ b/packages/svelte/src/compiler/phases/1-parse/state/element.js @@ -485,6 +485,25 @@ function read_static_attribute(parser) { function read_attribute(parser) { const start = parser.index; + if (parser.eat('#display={')) { + parser.allow_whitespace(); + const expression = read_expression(parser); + parser.allow_whitespace(); + parser.eat('}', true); + + /** @type {AST.DisplayDirective} */ + const display = { + type: 'DisplayDirective', + start, + end: parser.index, + expression, + metadata: { + expression: create_expression_metadata() + } + }; + return display; + } + if (parser.eat('{')) { parser.allow_whitespace(); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 2c2c287f1275..9e2b7c44931f 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -82,6 +82,12 @@ export function RegularElement(node, context) { /** @type {AST.StyleDirective[]} */ const style_directives = []; + /** @type {AST.DisplayDirective | null} */ + let display_directive = null; + + /** @type {AST.StyleDirective | null} */ + let style_display = null; + /** @type {Array} */ const other_directives = []; @@ -152,6 +158,19 @@ export function RegularElement(node, context) { has_use = true; other_directives.push(attribute); break; + + case 'DisplayDirective': + display_directive = attribute; + break; + } + } + + if (display_directive) { + // When we have a #display directive, the style:display directive must be handheld differently + const index = style_directives.findIndex((d) => d.name === 'display'); + if (index >= 0) { + style_display = style_directives[index]; + style_directives.splice(index, 1); } } @@ -403,7 +422,45 @@ export function RegularElement(node, context) { } } - if (node.fragment.nodes.some((node) => node.type === 'SnippetBlock')) { + if (display_directive) { + const block = b.block([ + ...child_state.init, + ...element_state.init, + child_state.update.length > 0 ? build_render_statement(child_state.update) : b.empty, + ...child_state.after_update, + ...element_state.after_update + ]); + + const visibility = b.thunk( + /** @type {Expression} */ (context.visit(display_directive.expression)) + ); + + /** @type {Expression | undefined} */ + let value = undefined; + + /** @type {Expression | undefined} */ + let important = undefined; + + if (style_display) { + value = + style_display.value === true + ? build_getter({ name: style_display.name, type: 'Identifier' }, context.state) + : build_attribute_value(style_display.value, context).value; + + if (style_display.metadata.expression.has_call) { + const id = b.id(state.scope.generate('style_directive')); + + state.init.push(b.const(id, create_derived(state, b.thunk(value)))); + value = b.call('$.get', id); + } + value = b.thunk(value); + important = style_display.modifiers.includes('important') ? b.true : undefined; + } + + context.state.init.push( + b.stmt(b.call('$.display', node_id, visibility, b.arrow([], block), value, important)) + ); + } else if (node.fragment.nodes.some((node) => node.type === 'SnippetBlock')) { // Wrap children in `{...}` to avoid declaration conflicts context.state.init.push( b.block([ diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index 5ebc6475713f..e6a17e968ddb 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -1,4 +1,4 @@ -/** @import { Expression, Literal } from 'estree' */ +/** @import { Expression, Literal, Property } from 'estree' */ /** @import { AST, Namespace } from '#compiler' */ /** @import { ComponentContext, ComponentServerTransformState } from '../../types.js' */ import { @@ -43,6 +43,9 @@ export function build_element_attributes(node, context) { /** @type {AST.StyleDirective[]} */ const style_directives = []; + /** @type {AST.DisplayDirective | null} */ + let display_directive = null; + /** @type {Expression | null} */ let content = null; @@ -189,6 +192,8 @@ export function build_element_attributes(node, context) { style_directives.push(attribute); } else if (attribute.type === 'LetDirective') { // do nothing, these are handled inside `build_inline_component` + } else if (attribute.type === 'DisplayDirective') { + display_directive = attribute; } else { context.visit(attribute); } @@ -204,8 +209,9 @@ export function build_element_attributes(node, context) { } } - if (style_directives.length > 0 && !has_spread) { + if ((display_directive !== null || style_directives.length > 0) && !has_spread) { build_style_directives( + display_directive, style_directives, /** @type {AST.Attribute | null} */ (attributes[style_index] ?? null), context @@ -216,7 +222,14 @@ export function build_element_attributes(node, context) { } if (has_spread) { - build_element_spread_attributes(node, attributes, style_directives, class_directives, context); + build_element_spread_attributes( + node, + attributes, + display_directive, + style_directives, + class_directives, + context + ); } else { for (const attribute of /** @type {AST.Attribute[]} */ (attributes)) { if (attribute.value === true || is_text_attribute(attribute)) { @@ -282,6 +295,7 @@ function get_attribute_name(element, attribute) { * * @param {AST.RegularElement | AST.SvelteElement} element * @param {Array} attributes + * @param {AST.DisplayDirective | null } display_directive * @param {AST.StyleDirective[]} style_directives * @param {AST.ClassDirective[]} class_directives * @param {ComponentContext} context @@ -289,6 +303,7 @@ function get_attribute_name(element, attribute) { function build_element_spread_attributes( element, attributes, + display_directive, style_directives, class_directives, context @@ -314,7 +329,7 @@ function build_element_spread_attributes( classes = b.object(properties); } - if (style_directives.length > 0) { + if (style_directives.length > 0 || display_directive !== null) { const properties = style_directives.map((directive) => b.init( directive.name, @@ -323,7 +338,7 @@ function build_element_spread_attributes( : build_attribute_value(directive.value, context, true) ) ); - + handle_display_directive(display_directive, properties, context); styles = b.object(properties); } @@ -402,11 +417,32 @@ function build_class_directives(class_directives, class_attribute) { } /** + * @param {AST.DisplayDirective | null} display_directive + * @param {Property[]} styles + * @param {ComponentContext} context + */ +function handle_display_directive(display_directive, styles, context) { + if (display_directive !== null) { + let display = styles.find((s) => s.key.type === 'Identifier' && s.key.name === 'display'); + if (display === undefined) { + display = b.init('display', b.literal(null)); + styles.push(display); + } + display.value = b.conditional( + /** @type {Expression} */ (context.visit(display_directive.expression)), + /** @type {Expression} */ (display.value), + b.literal('none !important') + ); + } +} + +/** + * @param {AST.DisplayDirective | null} display_directive * @param {AST.StyleDirective[]} style_directives * @param {AST.Attribute | null} style_attribute * @param {ComponentContext} context */ -function build_style_directives(style_directives, style_attribute, context) { +function build_style_directives(display_directive, style_directives, style_attribute, context) { const styles = style_directives.map((directive) => { let value = directive.value === true @@ -418,6 +454,8 @@ function build_style_directives(style_directives, style_attribute, context) { return b.init(directive.name, value); }); + handle_display_directive(display_directive, styles, context); + const arg = style_attribute === null ? b.object(styles) diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index 97a25df4a758..cafc9d6ead74 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -494,6 +494,15 @@ export namespace AST { }; } + export interface DisplayDirective extends BaseNode { + type: 'DisplayDirective'; + expression: Expression; + /** @internal */ + metadata: { + expression: ExpressionMetadata; + }; + } + export interface Script extends BaseNode { type: 'Script'; context: 'default' | 'module'; @@ -511,7 +520,8 @@ export namespace AST { | AST.OnDirective | AST.StyleDirective | AST.TransitionDirective - | AST.UseDirective; + | AST.UseDirective + | AST.DisplayDirective; export type Block = | AST.EachBlock diff --git a/packages/svelte/src/internal/client/dom/blocks/display.js b/packages/svelte/src/internal/client/dom/blocks/display.js new file mode 100644 index 000000000000..2272c63f57fb --- /dev/null +++ b/packages/svelte/src/internal/client/dom/blocks/display.js @@ -0,0 +1,80 @@ +import { UNINITIALIZED } from '../../../../constants.js'; +import { block, template_effect } from '../../reactivity/effects.js'; +import { hydrate_next, hydrate_node, hydrating } from '../hydration.js'; + +/** + * @template V + * @param {HTMLElement} node + * @param {() => any} get_visibility + * @param {() => void} render_fn + * @param {()=>string|null} [get_value] + * @param {boolean} [default_important] + * @returns {void} + */ +export function display(node, get_visibility, render_fn, get_value, default_important) { + if (hydrating) { + hydrate_next(); + } + + var anchor = node; + + /** @type {boolean | typeof UNINITIALIZED} */ + let prev_visible = UNINITIALIZED; + /** @type {string | null | undefined | typeof UNINITIALIZED} */ + let prev_value = UNINITIALIZED; + + const effect = block(render_fn); + template_effect(() => { + const visible = !!get_visibility(); + const value = visible ? get_value?.() : 'none'; + + if (visible === prev_visible && value === prev_value) { + return; + } + + const transitions = effect.transitions; + const run_transitions = prev_visible !== UNINITIALIZED && transitions?.length; + + if (visible || !run_transitions) { + if (value == null) { + anchor.style.removeProperty('display'); + } else { + anchor.style.setProperty( + 'display', + value, + !visible || default_important ? 'important' : '' + ); + } + } + + if (run_transitions) { + if (visible) { + // Start show transition + for (const transition of transitions) { + transition.in(); + } + } else { + var remaining = transitions.length; + var check = () => { + if (--remaining == 0) { + // cleanup + for (var transition of transitions) { + transition.stop(); + } + anchor.style.setProperty('display', 'none', 'important'); + } + }; + for (var transition of transitions) { + transition.out(check); + } + } + } + + prev_visible = visible; + prev_value = value; + }); + + if (hydrating) { + anchor = /** @type {HTMLElement} */ (hydrate_node); + } +} diff --git a/packages/svelte/src/internal/client/index.js b/packages/svelte/src/internal/client/index.js index f22c33babc52..f2aa494e56c0 100644 --- a/packages/svelte/src/internal/client/index.js +++ b/packages/svelte/src/internal/client/index.js @@ -17,6 +17,7 @@ export { inspect } from './dev/inspect.js'; export { await_block as await } from './dom/blocks/await.js'; export { if_block as if } from './dom/blocks/if.js'; export { key_block as key } from './dom/blocks/key.js'; +export { display } from './dom/blocks/display.js'; export { css_props } from './dom/blocks/css-props.js'; export { index, each } from './dom/blocks/each.js'; export { html } from './dom/blocks/html.js'; diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index d422abebbc0f..62d54f51dec5 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -1269,6 +1269,11 @@ declare module 'svelte/compiler' { expression: Expression; } + export interface DisplayDirective extends BaseNode { + type: 'DisplayDirective'; + expression: Expression; + } + export interface Script extends BaseNode { type: 'Script'; context: 'default' | 'module'; @@ -1286,7 +1291,8 @@ declare module 'svelte/compiler' { | AST.OnDirective | AST.StyleDirective | AST.TransitionDirective - | AST.UseDirective; + | AST.UseDirective + | AST.DisplayDirective; export type Block = | AST.EachBlock From 313a5614c394b6953343bbb4a7dc95387281b2be Mon Sep 17 00:00:00 2001 From: adiguba Date: Sat, 21 Dec 2024 21:16:43 +0100 Subject: [PATCH 2/6] support for svelte:element --- .../client/visitors/RegularElement.js | 54 ++++--------------- .../client/visitors/SvelteElement.js | 27 +++++++++- .../client/visitors/shared/element.js | 37 ++++++++++++- 3 files changed, 73 insertions(+), 45 deletions(-) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 9e2b7c44931f..58536d055390 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -1,4 +1,4 @@ -/** @import { Expression, ExpressionStatement, Identifier, MemberExpression, ObjectExpression, Statement } from 'estree' */ +/** @import { BlockStatement, Expression, ExpressionStatement, Identifier, MemberExpression, ObjectExpression, Statement } from 'estree' */ /** @import { AST } from '#compiler' */ /** @import { SourceLocation } from '#shared' */ /** @import { ComponentClientTransformState, ComponentContext } from '../types' */ @@ -22,7 +22,8 @@ import { build_attribute_value, build_class_directives, build_style_directives, - build_set_attributes + build_set_attributes, + build_display_directive } from './shared/element.js'; import { process_children } from './shared/fragment.js'; import { @@ -422,7 +423,8 @@ export function RegularElement(node, context) { } } - if (display_directive) { + if (display_directive || node.fragment.nodes.some((node) => node.type === 'SnippetBlock')) { + // Wrap children in `{...}` to avoid declaration conflicts const block = b.block([ ...child_state.init, ...element_state.init, @@ -430,47 +432,13 @@ export function RegularElement(node, context) { ...child_state.after_update, ...element_state.after_update ]); - - const visibility = b.thunk( - /** @type {Expression} */ (context.visit(display_directive.expression)) - ); - - /** @type {Expression | undefined} */ - let value = undefined; - - /** @type {Expression | undefined} */ - let important = undefined; - - if (style_display) { - value = - style_display.value === true - ? build_getter({ name: style_display.name, type: 'Identifier' }, context.state) - : build_attribute_value(style_display.value, context).value; - - if (style_display.metadata.expression.has_call) { - const id = b.id(state.scope.generate('style_directive')); - - state.init.push(b.const(id, create_derived(state, b.thunk(value)))); - value = b.call('$.get', id); - } - value = b.thunk(value); - important = style_display.modifiers.includes('important') ? b.true : undefined; + if (display_directive) { + context.state.init.push( + build_display_directive(node_id, display_directive, style_display, block, context) + ); + } else { + context.state.init.push(block); } - - context.state.init.push( - b.stmt(b.call('$.display', node_id, visibility, b.arrow([], block), value, important)) - ); - } else if (node.fragment.nodes.some((node) => node.type === 'SnippetBlock')) { - // Wrap children in `{...}` to avoid declaration conflicts - context.state.init.push( - b.block([ - ...child_state.init, - ...element_state.init, - child_state.update.length > 0 ? build_render_statement(child_state.update) : b.empty, - ...child_state.after_update, - ...element_state.after_update - ]) - ); } else if (node.fragment.metadata.dynamic) { context.state.init.push(...child_state.init, ...element_state.init); context.state.update.push(...child_state.update); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js index ba66fe29d691..5d87bf7edad0 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/SvelteElement.js @@ -12,6 +12,7 @@ import { determine_namespace_for_children } from '../../utils.js'; import { build_attribute_value, build_class_directives, + build_display_directive, build_set_attributes, build_style_directives } from './shared/element.js'; @@ -36,6 +37,12 @@ export function SvelteElement(node, context) { /** @type {AST.StyleDirective[]} */ const style_directives = []; + /** @type {AST.DisplayDirective | null} */ + let display_directive = null; + + /** @type {AST.StyleDirective | null} */ + let style_display = null; + /** @type {ExpressionStatement[]} */ const lets = []; @@ -73,11 +80,21 @@ export function SvelteElement(node, context) { } else if (attribute.type === 'OnDirective') { const handler = /** @type {Expression} */ (context.visit(attribute, inner_context.state)); inner_context.state.after_update.push(b.stmt(handler)); + } else if (attribute.type === 'DisplayDirective') { + display_directive = attribute; } else { context.visit(attribute, inner_context.state); } } + if (display_directive !== null) { + const idx = style_directives.findIndex((d) => d.name === 'display'); + if (idx > 0) { + style_display = style_directives[idx]; + style_directives.splice(idx, 1); + } + } + // Let bindings first, they can be used on attributes context.state.init.push(...lets); // create computeds in the outer context; the dynamic element is the single child of this slot @@ -140,6 +157,14 @@ export function SvelteElement(node, context) { const location = dev && locator(node.start); + let render_element = b.block(inner); + + if (display_directive) { + render_element = b.block([ + build_display_directive(element_id, display_directive, style_display, render_element, context) + ]); + } + context.state.init.push( b.stmt( b.call( @@ -147,7 +172,7 @@ export function SvelteElement(node, context) { context.state.node, get_tag, node.metadata.svg || node.metadata.mathml ? b.true : b.false, - inner.length > 0 && b.arrow([element_id, b.id('$$anchor')], b.block(inner)), + inner.length > 0 && b.arrow([element_id, b.id('$$anchor')], render_element), dynamic_namespace && b.thunk(build_attribute_value(dynamic_namespace, context).value), location && b.array([b.literal(location.line), b.literal(location.column)]) ) diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js index 1b0737e31e18..e3feaf2ca04c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js @@ -1,4 +1,4 @@ -/** @import { Expression, Identifier, ObjectExpression } from 'estree' */ +/** @import { BlockStatement, Expression, Identifier, ObjectExpression } from 'estree' */ /** @import { AST, Namespace } from '#compiler' */ /** @import { ComponentClientTransformState, ComponentContext } from '../../types' */ import { normalize_attribute } from '../../../../../../utils.js'; @@ -214,3 +214,38 @@ export function get_attribute_name(element, attribute) { return attribute.name; } + +/** + * @param {Identifier} node_id + * @param {AST.DisplayDirective} display + * @param {AST.StyleDirective | null} style + * @param {BlockStatement} block + * @param {ComponentContext} context + */ +export function build_display_directive(node_id, display, style, block, context) { + const visibility = b.thunk(/** @type {Expression} */ (context.visit(display.expression))); + + /** @type {Expression | undefined} */ + let value = undefined; + + /** @type {Expression | undefined} */ + let important = undefined; + + if (style) { + value = + style.value === true + ? build_getter({ name: style.name, type: 'Identifier' }, context.state) + : build_attribute_value(style.value, context).value; + + if (style.metadata.expression.has_call) { + const id = b.id(context.state.scope.generate('style_directive')); + + context.state.init.push(b.const(id, create_derived(context.state, b.thunk(value)))); + value = b.call('$.get', id); + } + value = b.thunk(value); + important = style.modifiers.includes('important') ? b.true : undefined; + } + + return b.stmt(b.call('$.display', node_id, visibility, b.arrow([], block), value, important)); +} From 272f7d4dfc08443a9dddd685f909fbf59d224fd5 Mon Sep 17 00:00:00 2001 From: adiguba Date: Sat, 21 Dec 2024 21:39:21 +0100 Subject: [PATCH 3/6] handle for style="" when there no style:display --- .../src/internal/client/dom/blocks/display.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/svelte/src/internal/client/dom/blocks/display.js b/packages/svelte/src/internal/client/dom/blocks/display.js index 2272c63f57fb..1ac813fb3fe4 100644 --- a/packages/svelte/src/internal/client/dom/blocks/display.js +++ b/packages/svelte/src/internal/client/dom/blocks/display.js @@ -23,6 +23,22 @@ export function display(node, get_visibility, render_fn, get_value, default_impo /** @type {string | null | undefined | typeof UNINITIALIZED} */ let prev_value = UNINITIALIZED; + // special case when style:display is missing + // we get the style value from the node : + if (get_value === undefined) { + /** @type {string | null} */ + let anchor_display = null; + get_value = () => { + if (prev_visible !== false) { + anchor_display = anchor.style.display || null; + if (anchor_display) { + default_important = anchor.style.getPropertyPriority('display') === 'important'; + } + } + return anchor_display; + }; + } + const effect = block(render_fn); template_effect(() => { const visible = !!get_visibility(); From 42bdeaaaa92fac647486bcafaf80e4b8119c1a62 Mon Sep 17 00:00:00 2001 From: adiguba Date: Thu, 26 Dec 2024 09:14:35 +0100 Subject: [PATCH 4/6] basic support for style:display|transition --- .../2-analyze/visitors/StyleDirective.js | 7 ++++++- .../client/visitors/RegularElement.js | 18 +++++++++++++++--- .../client/visitors/shared/element.js | 6 ++++-- .../svelte/src/compiler/types/template.d.ts | 2 +- .../src/internal/client/dom/blocks/display.js | 12 +++++++++--- 5 files changed, 35 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js index 7d6eb5be99e8..3aff246b067e 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/StyleDirective.js @@ -10,7 +10,12 @@ import { mark_subtree_dynamic } from './shared/fragment.js'; */ export function StyleDirective(node, context) { if (node.modifiers.length > 1 || (node.modifiers.length && node.modifiers[0] !== 'important')) { - e.style_directive_invalid_modifier(node); + if ( + node.name !== 'display' || + node.modifiers.findIndex((m) => m !== 'important' && m !== 'transition') >= 0 + ) { + e.style_directive_invalid_modifier(node); + } } mark_subtree_dynamic(context.path); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 58536d055390..38792022f4fa 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -148,7 +148,11 @@ export function RegularElement(node, context) { break; case 'StyleDirective': - style_directives.push(attribute); + if (attribute.name === 'display' && attribute.modifiers.includes('transition')) { + style_display = attribute; + } else { + style_directives.push(attribute); + } break; case 'TransitionDirective': @@ -167,6 +171,10 @@ export function RegularElement(node, context) { } if (display_directive) { + if (style_display !== null) { + // TODO + throw new Error('#display and style:display|transition forbidden'); + } // When we have a #display directive, the style:display directive must be handheld differently const index = style_directives.findIndex((d) => d.name === 'display'); if (index >= 0) { @@ -423,7 +431,11 @@ export function RegularElement(node, context) { } } - if (display_directive || node.fragment.nodes.some((node) => node.type === 'SnippetBlock')) { + if ( + style_display || + display_directive || + node.fragment.nodes.some((node) => node.type === 'SnippetBlock') + ) { // Wrap children in `{...}` to avoid declaration conflicts const block = b.block([ ...child_state.init, @@ -432,7 +444,7 @@ export function RegularElement(node, context) { ...child_state.after_update, ...element_state.after_update ]); - if (display_directive) { + if (display_directive || style_display) { context.state.init.push( build_display_directive(node_id, display_directive, style_display, block, context) ); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js index e3feaf2ca04c..54e08958c5f7 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/element.js @@ -217,13 +217,15 @@ export function get_attribute_name(element, attribute) { /** * @param {Identifier} node_id - * @param {AST.DisplayDirective} display + * @param {AST.DisplayDirective | null} display * @param {AST.StyleDirective | null} style * @param {BlockStatement} block * @param {ComponentContext} context */ export function build_display_directive(node_id, display, style, block, context) { - const visibility = b.thunk(/** @type {Expression} */ (context.visit(display.expression))); + const visibility = display + ? b.thunk(/** @type {Expression} */ (context.visit(display.expression))) + : b.literal(null); /** @type {Expression | undefined} */ let value = undefined; diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index cafc9d6ead74..23f96bae03e9 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -240,7 +240,7 @@ export namespace AST { name: string; /** The 'y' in `style:x={y}` */ value: true | ExpressionTag | Array; - modifiers: Array<'important'>; + modifiers: Array<'important' | 'transition'>; /** @internal */ metadata: { expression: ExpressionMetadata; diff --git a/packages/svelte/src/internal/client/dom/blocks/display.js b/packages/svelte/src/internal/client/dom/blocks/display.js index 1ac813fb3fe4..337931af3725 100644 --- a/packages/svelte/src/internal/client/dom/blocks/display.js +++ b/packages/svelte/src/internal/client/dom/blocks/display.js @@ -7,7 +7,7 @@ import { hydrate_next, hydrate_node, hydrating } from '../hydration.js'; * @param {HTMLElement} node * @param {() => any} get_visibility * @param {() => void} render_fn - * @param {()=>string|null} [get_value] + * @param {()=>string|boolean|null} [get_value] * @param {boolean} [default_important] * @returns {void} */ @@ -41,8 +41,14 @@ export function display(node, get_visibility, render_fn, get_value, default_impo const effect = block(render_fn); template_effect(() => { - const visible = !!get_visibility(); - const value = visible ? get_value?.() : 'none'; + let display_value = get_value?.(); + if (display_value === true) { + display_value = null; + } else if (display_value === false) { + display_value = 'none'; + } + const visible = get_visibility !== null ? !!get_visibility() : display_value !== 'none'; + const value = visible ? display_value : 'none'; if (visible === prev_visible && value === prev_value) { return; From 9f1400ef994382e8d2b305bfcdc1bfecfdd4f987 Mon Sep 17 00:00:00 2001 From: adiguba Date: Fri, 27 Dec 2024 08:44:56 +0100 Subject: [PATCH 5/6] rebuild type --- packages/svelte/types/index.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts index 62d54f51dec5..5af0b30ed34e 100644 --- a/packages/svelte/types/index.d.ts +++ b/packages/svelte/types/index.d.ts @@ -1103,7 +1103,7 @@ declare module 'svelte/compiler' { name: string; /** The 'y' in `style:x={y}` */ value: true | ExpressionTag | Array; - modifiers: Array<'important'>; + modifiers: Array<'important' | 'transition'>; } // TODO have separate in/out/transition directives From dcebd46c5e1860f3918c815bc8098598d32c5633 Mon Sep 17 00:00:00 2001 From: adiguba Date: Fri, 27 Dec 2024 08:45:29 +0100 Subject: [PATCH 6/6] support for style:display|transition with boolean value in SSR --- .../phases/3-transform/server/visitors/shared/element.js | 3 +++ packages/svelte/src/internal/server/index.js | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index e6a17e968ddb..48d614b5e484 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -448,6 +448,9 @@ function build_style_directives(display_directive, style_directives, style_attri directive.value === true ? b.id(directive.name) : build_attribute_value(directive.value, context, true); + if (directive.name === 'display' && directive.modifiers.includes('transition')) { + value = b.call('$.get_display', value); + } if (directive.modifiers.includes('important')) { value = b.binary('+', value, b.literal(' !important')); } diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index b8371b7e008f..8e36ed50d45d 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -270,6 +270,13 @@ function style_object_to_string(style_object) { .join(' '); } +/** @param {string|boolean} value */ +export function get_display(value) { + if (value === true) return null; + if (value === false) return 'none'; + return value; +} + /** @param {Record} style_object */ export function add_styles(style_object) { const styles = style_object_to_string(style_object);