From bfee89364b790bdd9016578f0780a2e672e79a42 Mon Sep 17 00:00:00 2001 From: Marlus Saraiva Date: Fri, 14 Oct 2022 15:19:04 -0300 Subject: [PATCH] Optimized static props for diff tracking (#665) --- lib/surface/compiler/eex_engine.ex | 113 +++++++++++++----- .../components/dynamic/component_test.exs | 4 +- .../dynamic/live_component_test.exs | 4 +- .../integrations/lv_change_tracking_test.exs | 23 ++++ test/surface/integrations/properties_test.exs | 25 +--- 5 files changed, 110 insertions(+), 59 deletions(-) diff --git a/lib/surface/compiler/eex_engine.ex b/lib/surface/compiler/eex_engine.ex index cd86c3190..29f85bc9c 100644 --- a/lib/surface/compiler/eex_engine.ex +++ b/lib/surface/compiler/eex_engine.ex @@ -256,11 +256,11 @@ defmodule Surface.Compiler.EExEngine do %AST.AttributeExpr{value: expr} -> expr end - {props_expr, dynamic_props_expr} = build_props_expressions(component) + ctx = Surface.AST.Meta.quoted_caller_context(meta) + {static_props, props_expr, dynamic_props_expr} = build_props_expressions(component, ctx) {context_expr, context_var, state} = process_context(nil, nil, props, component, state) slot_props = build_slot_props(component, buffer, state, context_var) - slot_props_map = {:%{}, [], slot_props} - ctx = Surface.AST.Meta.quoted_caller_context(meta) + static_props_map = {:%{}, [], slot_props ++ static_props} quote do Phoenix.LiveView.HTMLEngine.component( @@ -274,7 +274,7 @@ defmodule Surface.Compiler.EExEngine do unquote(meta.node_alias), unquote(ctx) ), - unquote(slot_props_map) + unquote(static_props_map) ), {__MODULE__, __ENV__.function, __ENV__.file, unquote(meta.line)} ) @@ -286,11 +286,11 @@ defmodule Surface.Compiler.EExEngine do defp to_expression(%AST.FunctionComponent{type: :local} = component, buffer, state) do %AST.FunctionComponent{module: module, fun: fun, props: props, meta: meta} = component - {props_expr, dynamic_props_expr} = build_props_expressions(component) + ctx = Surface.AST.Meta.quoted_caller_context(meta) + {static_props, props_expr, dynamic_props_expr} = build_props_expressions(component, ctx) {context_expr, context_var, state} = process_context(module, fun, props, component, state) slot_props = build_slot_props(component, buffer, state, context_var) - slot_props_map = {:%{}, [], slot_props} - ctx = Surface.AST.Meta.quoted_caller_context(meta) + static_props_map = {:%{}, [], slot_props ++ static_props} quote do Phoenix.LiveView.HTMLEngine.component( @@ -304,7 +304,7 @@ defmodule Surface.Compiler.EExEngine do unquote(meta.node_alias), unquote(ctx) ), - unquote(slot_props_map) + unquote(static_props_map) ), {__MODULE__, __ENV__.function, __ENV__.file, unquote(meta.line)} ) @@ -316,11 +316,11 @@ defmodule Surface.Compiler.EExEngine do defp to_expression(%AST.FunctionComponent{type: :remote} = component, buffer, state) do %AST.FunctionComponent{module: module, fun: fun, props: props, meta: meta} = component - {props_expr, dynamic_props_expr} = build_props_expressions(component) + ctx = Surface.AST.Meta.quoted_caller_context(meta) + {static_props, props_expr, dynamic_props_expr} = build_props_expressions(component, ctx) {context_expr, context_var, state} = process_context(module, fun, props, component, state) slot_props = build_slot_props(component, buffer, state, context_var) - slot_props_map = {:%{}, [], slot_props} - ctx = Surface.AST.Meta.quoted_caller_context(meta) + static_props_map = {:%{}, [], slot_props ++ static_props} # For now, we can only retrieve props and slots information from module components, # not function components, so if we're dealing with dynamic or recursive module components, @@ -339,7 +339,7 @@ defmodule Surface.Compiler.EExEngine do unquote(meta.node_alias), unquote(ctx) ), - unquote(slot_props_map) + unquote(static_props_map) ), {__MODULE__, __ENV__.function, __ENV__.file, unquote(meta.line)} ) @@ -351,11 +351,11 @@ defmodule Surface.Compiler.EExEngine do defp to_expression(%AST.Component{type: Surface.Component} = component, buffer, state) do %AST.Component{module: module, props: props, meta: meta} = component - {props_expr, dynamic_props_expr} = build_props_expressions(component) + ctx = Surface.AST.Meta.quoted_caller_context(meta) + {static_props, props_expr, dynamic_props_expr} = build_props_expressions(component, ctx) {context_expr, context_var, state} = process_context(module, :render, props, component, state) slot_props = build_slot_props(component, buffer, state, context_var) - slot_props_map = {:%{}, [], slot_props} - ctx = Surface.AST.Meta.quoted_caller_context(meta) + static_props_map = {:%{}, [], static_props ++ slot_props} quote do Phoenix.LiveView.HTMLEngine.component( @@ -369,7 +369,7 @@ defmodule Surface.Compiler.EExEngine do unquote(meta.node_alias), unquote(ctx) ), - unquote(slot_props_map) + unquote(static_props_map) ), {__MODULE__, __ENV__.function, __ENV__.file, unquote(meta.line)} ) @@ -381,11 +381,11 @@ defmodule Surface.Compiler.EExEngine do defp to_expression(%AST.SlotableComponent{} = component, buffer, state) do %AST.SlotableComponent{module: module, props: props, meta: meta} = component - {props_expr, dynamic_props_expr} = build_props_expressions(component) + ctx = Surface.AST.Meta.quoted_caller_context(meta) + {static_props, props_expr, dynamic_props_expr} = build_props_expressions(component, ctx) {context_expr, context_var, state} = process_context(module, :render, props, component, state) slot_props = build_slot_props(component, buffer, state, context_var) - slot_props_map = {:%{}, [], slot_props} - ctx = Surface.AST.Meta.quoted_caller_context(meta) + static_props_map = {:%{}, [], slot_props ++ static_props} quote do Phoenix.LiveView.HTMLEngine.component( @@ -399,7 +399,7 @@ defmodule Surface.Compiler.EExEngine do unquote(meta.node_alias), unquote(ctx) ), - unquote(slot_props_map) + unquote(static_props_map) ), {__MODULE__, __ENV__.function, __ENV__.file, unquote(meta.line)} ) @@ -411,11 +411,11 @@ defmodule Surface.Compiler.EExEngine do defp to_expression(%AST.Component{type: Surface.LiveComponent} = component, buffer, state) do %AST.Component{module: module, props: props, meta: meta} = component - {props_expr, dynamic_props_expr} = build_props_expressions(component) + ctx = Surface.AST.Meta.quoted_caller_context(meta) + {static_props, props_expr, dynamic_props_expr} = build_props_expressions(component, ctx) {context_expr, context_var, state} = process_context(module, :render, props, component, state) slot_props = build_slot_props(component, buffer, state, context_var) - slot_props_map = {:%{}, [], [{:module, module} | slot_props]} - ctx = Surface.AST.Meta.quoted_caller_context(meta) + static_props_map = {:%{}, [], [{:module, module} | slot_props] ++ static_props} quote do Phoenix.LiveView.HTMLEngine.component( @@ -429,7 +429,7 @@ defmodule Surface.Compiler.EExEngine do unquote(meta.node_alias), unquote(ctx) ), - unquote(slot_props_map) + unquote(static_props_map) ), {__MODULE__, __ENV__.function, __ENV__.file, unquote(meta.line)} ) @@ -445,11 +445,11 @@ defmodule Surface.Compiler.EExEngine do meta: meta } = component - {props_expr, dynamic_props_expr} = build_props_expressions(component) + ctx = Surface.AST.Meta.quoted_caller_context(meta) + {static_props, props_expr, dynamic_props_expr} = build_props_expressions(component, ctx) {context_expr, context_var, state} = process_context(nil, :render, props, component, state) slot_props = build_slot_props(component, buffer, state, context_var) - slot_props_map = {:%{}, [], [{:module, module_expr} | slot_props]} - ctx = Surface.AST.Meta.quoted_caller_context(meta) + static_props_map = {:%{}, [], [{:module, module_expr} | slot_props] ++ static_props} quote do Phoenix.LiveView.HTMLEngine.component( @@ -463,7 +463,7 @@ defmodule Surface.Compiler.EExEngine do unquote(meta.node_alias), unquote(ctx) ), - unquote(slot_props_map) + unquote(static_props_map) ), {__MODULE__, __ENV__.function, __ENV__.file, unquote(meta.line)} ) @@ -1277,11 +1277,62 @@ defmodule Surface.Compiler.EExEngine do end end - defp build_props_expressions(%{props: props, dynamic_props: dynamic_props}) do - props_expr = collect_component_props(props) + defp build_props_expressions(%{module: module, props: attrs, dynamic_props: dynamic_props, meta: meta}, ctx) do + module_expr = + case module do + %AST.AttributeExpr{value: module_expr} -> module_expr + _ -> module + end + dynamic_props_expr = handle_dynamic_props(dynamic_props) - {props_expr, dynamic_props_expr} + {props_expr, props_with_dynamic_value} = + Enum.reduce(attrs, {[], MapSet.new()}, fn attr, {props, props_with_dynamic_value} -> + %AST.Attribute{root: root, value: expr} = attr + value = to_prop_expr(expr) + prop_name = if root, do: :__root__, else: attr.name + + # We consider a prop as dynamic if it has at least one dynamic value + # TODO: use `Macro.quoted_literal?/1` instead + props_with_dynamic_value = + if is_binary(value) do + props_with_dynamic_value + else + MapSet.put(props_with_dynamic_value, prop_name) + end + + {[{prop_name, value} | props], props_with_dynamic_value} + end) + + # We can't have this in the reduce above as we need the final `props_with_dynamic_value` + {props_expr, static_props} = + Enum.reduce(props_expr, {[], %{}}, fn {k, v}, {dynamic, static} -> + if MapSet.member?(props_with_dynamic_value, k) do + {[{k, v} | dynamic], static} + else + {dynamic, Map.put(static, k, [v | static[k] || []])} + end + end) + + static_props = + for {k, v} <- static_props do + ast = + quote do + Surface.TypeHandler.runtime_prop_value!( + unquote(module_expr), + unquote(k), + unquote(v), + [], + unquote(meta.node_alias), + nil, + unquote(ctx) + ) + end + + {k, ast} + end + + {static_props, props_expr, dynamic_props_expr} end defp make_bindings_ast_generated(ast) do diff --git a/test/surface/components/dynamic/component_test.exs b/test/surface/components/dynamic/component_test.exs index f8dd2260b..abb2bd6d9 100644 --- a/test/surface/components/dynamic/component_test.exs +++ b/test/surface/components/dynamic/component_test.exs @@ -418,7 +418,7 @@ defmodule Surface.Components.Dynamic.ComponentTest do """ end - test "at runtime, warn on unknown attributes at the component definition's file/line " do + test "at runtime, warn on unknown attributes with dynamic values at the component definition's file/line " do file = Path.relative_to_cwd(__ENV__.file) line = __ENV__.line + 8 @@ -430,7 +430,7 @@ defmodule Surface.Components.Dynamic.ComponentTest do ~F""" """ end diff --git a/test/surface/components/dynamic/live_component_test.exs b/test/surface/components/dynamic/live_component_test.exs index ad899c74d..b5cf3e06b 100644 --- a/test/surface/components/dynamic/live_component_test.exs +++ b/test/surface/components/dynamic/live_component_test.exs @@ -161,7 +161,7 @@ defmodule Surface.Components.Dynamic.LiveComponentTest do """ end - test "at runtime, warn on unknown attributes at the component definition's file/line " do + test "at runtime, warn on unknown attributes with dynamic values at the component definition's file/line " do file = Path.relative_to_cwd(__ENV__.file) line = __ENV__.line + 8 @@ -174,7 +174,7 @@ defmodule Surface.Components.Dynamic.LiveComponentTest do """ end diff --git a/test/surface/integrations/lv_change_tracking_test.exs b/test/surface/integrations/lv_change_tracking_test.exs index 0a478b7dc..868453bc9 100644 --- a/test/surface/integrations/lv_change_tracking_test.exs +++ b/test/surface/integrations/lv_change_tracking_test.exs @@ -112,6 +112,29 @@ defmodule Surface.LVChangeTrackingTest do assert has_dynamic_part?(full_render, "INNER WITH ARG") end + test "static surface props are not resent after first rendering" do + import Surface + + assigns = %{socket: %Socket{}, content: "DYN CONTENT"} + + comp = fn assigns -> + ~F""" + <.inner label="STATIC LABEL" content={@content} {...dyn: 1}/> + """ + end + + {socket, full_render, components} = render(comp.(assigns)) + + assert has_dynamic_part?(full_render, "STATIC LABEL") + + assigns = Map.put(assigns, :__changed__, %{content: true}) + + {_, full_render, _} = render(comp.(assigns), socket.fingerprints, components) + + assert has_dynamic_part?(full_render, "DYN CONTENT") + refute has_dynamic_part?(full_render, "STATIC LABEL") + end + defp render( rendered, fingerprints \\ Diff.new_fingerprints(), diff --git a/test/surface/integrations/properties_test.exs b/test/surface/integrations/properties_test.exs index 1d30a6062..7216ba947 100644 --- a/test/surface/integrations/properties_test.exs +++ b/test/surface/integrations/properties_test.exs @@ -834,7 +834,6 @@ defmodule Surface.PropertiesSyncTest do import ExUnit.CaptureIO alias Surface.PropertiesTest.StringProp - alias Surface.PropertiesTest.RootProp alias Surface.PropertiesTest.ListProp test "warn if prop is required and has default value" do @@ -983,6 +982,7 @@ defmodule Surface.PropertiesSyncTest do end) end + # TODO: remove it when start using `attr` from LV describe "unknown property" do test "warns at runtime and render the component" do output = @@ -1148,29 +1148,6 @@ defmodule Surface.PropertiesSyncTest do """ end - test "if not true renders only the last value, root prop" do - output = - capture_io(:standard_error, fn -> - html = - render_surface do - ~F""" - - """ - end - - assert html =~ """ - attr label - """ - end) - - assert output =~ """ - the prop `label` has been passed multiple times. Considering only the last value. - - Hint: Either specify the `label` via the root property (``) or \ - explicitly via the label property (``), but not both. - """ - end - test "warns without root prop" do output = capture_io(:standard_error, fn ->