Skip to content

Commit

Permalink
Optimized static props for diff tracking (#665)
Browse files Browse the repository at this point in the history
  • Loading branch information
msaraiva authored Oct 14, 2022
1 parent 1590b75 commit bfee893
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 59 deletions.
113 changes: 82 additions & 31 deletions lib/surface/compiler/eex_engine.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)}
)
Expand All @@ -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(
Expand All @@ -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)}
)
Expand All @@ -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,
Expand All @@ -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)}
)
Expand All @@ -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(
Expand All @@ -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)}
)
Expand All @@ -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(
Expand All @@ -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)}
)
Expand All @@ -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(
Expand All @@ -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)}
)
Expand All @@ -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(
Expand All @@ -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)}
)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions test/surface/components/dynamic/component_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -430,7 +430,7 @@ defmodule Surface.Components.Dynamic.ComponentTest do
~F"""
<Component
module={@mod}
unknown_attr="123"
unknown_attr={"123"}
/>
"""
end
Expand Down
4 changes: 2 additions & 2 deletions test/surface/components/dynamic/live_component_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -174,7 +174,7 @@ defmodule Surface.Components.Dynamic.LiveComponentTest do
<LiveComponent
module={@mod}
id="comp"
unknown_attr="123"
unknown_attr={"123"}
/>
"""
end
Expand Down
23 changes: 23 additions & 0 deletions test/surface/integrations/lv_change_tracking_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
25 changes: 1 addition & 24 deletions test/surface/integrations/properties_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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"""
<RootProp {"root label"} label="attr label" />
"""
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 (`<RootProp { ... }>`) or \
explicitly via the label property (`<RootProp label="...">`), but not both.
"""
end

test "warns without root prop" do
output =
capture_io(:standard_error, fn ->
Expand Down

0 comments on commit bfee893

Please sign in to comment.