From bea1aeef7008a8618e737fc8e38f9c2975bdc30f Mon Sep 17 00:00:00 2001 From: Marlus Saraiva Date: Wed, 3 Jan 2024 16:27:22 -0300 Subject: [PATCH 1/7] Fix Malformed HTML in rendered.js --- lib/surface/compiler/eex_engine.ex | 22 ++++- test/surface/compiler_test.exs | 91 +++++++++++++++++++ .../components/live_file_input_test.exs | 4 + 3 files changed, 115 insertions(+), 2 deletions(-) diff --git a/lib/surface/compiler/eex_engine.ex b/lib/surface/compiler/eex_engine.ex index 4b21ea8c5..3140fa2fc 100644 --- a/lib/surface/compiler/eex_engine.ex +++ b/lib/surface/compiler/eex_engine.ex @@ -7,6 +7,7 @@ defmodule Surface.Compiler.EExEngine do for information on this). Finally, it passes these tokens into the engine sequentially in the same manner as EEx.Compiler.compile/2 """ + alias Surface.Compiler.Helpers alias Surface.AST alias Surface.IOHelper alias Surface.Components.Context @@ -27,7 +28,8 @@ defmodule Surface.Compiler.EExEngine do engine: opts[:engine] || @default_engine, depth: 0, context_vars: %{count: 0, changed: []}, - scope: [] + scope: [], + root_tag?: root_tag?(nodes) } nodes @@ -40,6 +42,22 @@ defmodule Surface.Compiler.EExEngine do ) end + defp root_tag?(nodes) do + Enum.reduce_while(nodes, false, fn + %AST.Tag{}, false -> + {:cont, true} + + %AST.Tag{}, true -> + {:halt, false} + + %AST.Literal{value: value}, acc -> + if Helpers.blank?(value), do: {:cont, acc}, else: {:halt, false} + + _node, _acc -> + {:halt, false} + end) + end + defp to_token_sequence(nodes) do nodes |> to_dynamic_nested_html() @@ -48,7 +66,7 @@ defmodule Surface.Compiler.EExEngine do end defp generate_buffer([], buffer, state) do - ast = state.engine.handle_body(buffer, root: true) + ast = state.engine.handle_body(buffer, root: state.root_tag?) quote do require Phoenix.LiveView.TagEngine diff --git a/test/surface/compiler_test.exs b/test/surface/compiler_test.exs index a49affb82..3b83aae9d 100644 --- a/test/surface/compiler_test.exs +++ b/test/surface/compiler_test.exs @@ -1,6 +1,10 @@ defmodule Surface.CompilerTest do use ExUnit.Case + import Surface, only: [sigil_F: 2] + import Phoenix.Component, only: [sigil_H: 2] + alias Phoenix.LiveView.Rendered + defmodule Macro do use Surface.MacroComponent @@ -93,6 +97,93 @@ defmodule Surface.CompilerTest do end end + defp func(assigns) do + ~F[hello] + end + + describe "set the root field for the %Phoenix.LiveView.Rendered{} struct" do + test "set it to `true` if the there's only a single root tag node" do + assigns = %{} + + assert %Rendered{root: true} = ~H[
] + assert %Rendered{root: true} = ~F[
] + assert %Rendered{root: true} = ~H[
text
] + assert %Rendered{root: true} = ~F[
text
] + assert %Rendered{root: true} = ~H[
<.func/>
] + assert %Rendered{root: true} = ~F[
<.func/>
] + assert %Rendered{root: true} = ~H[
<%= "text" %>
] + assert %Rendered{root: true} = ~F[
{"text"}
] + + assert %Rendered{root: true} = ~H[
text
] + assert %Rendered{root: true} = ~F[
text
] + + assert %Rendered{root: true} = ~H""" +
text
+ """ + + assert %Rendered{root: true} = ~F""" +
text
+ """ + end + + test "set it to `false` if the root tag is translated/wrapped into an expression e.g. using `:for` or `:if`" do + assigns = %{} + + assert %Rendered{root: false} = ~H[
] + assert %Rendered{root: false} = ~F[
] + assert %Rendered{root: false} = ~H(
) + assert %Rendered{root: false} = ~F(
) + end + + test "set it to `false` if it's a text node" do + assigns = %{} + + assert %Rendered{root: false} = ~H[text] + assert %Rendered{root: false} = ~F[text] + end + + test "set it to `false` if it's an expression" do + assigns = %{} + + assert %Rendered{root: false} = ~H[<%= "text" %>] + assert %Rendered{root: false} = ~F[{"text"}] + end + + test "set it to `false` if it's a component" do + assigns = %{} + + assert %Rendered{root: false} = ~H[<.func/>] + assert %Rendered{root: false} = ~F[<.func/>] + end + + test "set it to `false` if there are multiple nodes" do + assigns = %{} + + assert %Rendered{root: false} = ~H[
] + assert %Rendered{root: false} = ~F[
] + + assert %Rendered{root: false} = ~H[text
] + assert %Rendered{root: false} = ~F[text
] + + assert %Rendered{root: false} = ~H[
text] + assert %Rendered{root: false} = ~F[
text] + + assert %Rendered{root: false} = ~H[
<%= "text" %>] + assert %Rendered{root: false} = ~F[
{"text"}] + + assert %Rendered{root: false} = ~H[
] + assert %Rendered{root: false} = ~F[
] + + assert %Rendered{root: false} = ~H[
] + assert %Rendered{root: false} = ~F[
] + + # Private comments are excluded from the AST. + # And since they are not available in ~H, we don't assert against it. + assert %Rendered{root: true} = ~F[{!-- comment --}
] + assert %Rendered{root: true} = ~F[
{!-- comment --}] + end + end + test "show public comments" do code = """
diff --git a/test/surface/components/live_file_input_test.exs b/test/surface/components/live_file_input_test.exs index 9cf71fb45..62e28db14 100644 --- a/test/surface/components/live_file_input_test.exs +++ b/test/surface/components/live_file_input_test.exs @@ -16,7 +16,9 @@ defmodule Surface.Components.LiveFileInputTest do def render(assigns) do ~F""" +
+
""" end end @@ -34,7 +36,9 @@ defmodule Surface.Components.LiveFileInputTest do def render(assigns) do ~F""" +
+
""" end end From 54c5a02c2a84f619066b59c260e44574b89c9aab Mon Sep 17 00:00:00 2001 From: Marlus Saraiva Date: Wed, 3 Jan 2024 19:33:38 -0300 Subject: [PATCH 2/7] Fix error when passing special chars as literals inside an expression --- lib/surface/type_handler.ex | 7 ++++- test/surface/integrations/html_tag_test.exs | 29 ++++++++++++++++++--- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/lib/surface/type_handler.ex b/lib/surface/type_handler.ex index e0a598d62..fdd7f9dd1 100644 --- a/lib/surface/type_handler.ex +++ b/lib/surface/type_handler.ex @@ -177,7 +177,12 @@ defmodule Surface.TypeHandler do {:ok, [~S( ), to_string(name)]} {:ok, val} -> - {:ok, Phoenix.HTML.attributes_escape([{name, val}])} + attr_value = + [{name, val}] + |> Phoenix.HTML.attributes_escape() + |> Phoenix.HTML.safe_to_string() + + {:ok, attr_value} {:error, message} -> {:error, message} diff --git a/test/surface/integrations/html_tag_test.exs b/test/surface/integrations/html_tag_test.exs index 1003ef4e9..b7dc66ab2 100644 --- a/test/surface/integrations/html_tag_test.exs +++ b/test/surface/integrations/html_tag_test.exs @@ -143,6 +143,19 @@ defmodule HtmlTagTest do """ end + test "as expression with a literal string, encode HTML entities" do + html = + render_surface do + ~F""" +
123"}/> + """ + end + + assert html =~ """ +
+ """ + end + test "without a value" do html = render_surface do @@ -283,10 +296,20 @@ defmodule HtmlTagTest do """ end - test "as string literal, it's translated directly to static html" do - %Rendered{static: static} = eval(~S[
], __ENV__) + test "as string literal, translate directly to static html, without encoding" do + %Rendered{static: static} = eval(~S{
}, __ENV__) + + assert static == [~S{
}] + end + + test "as expression with a literal string, translate and encode directly to static html" do + code = """ +
\ + """ + + %Rendered{static: static} = eval(code, __ENV__) - assert static == [~S(
"] + assert static == [~S{
}] end test "css class with keyword list notation" do From d0c1512d8b00db4e2516f3c62e7027e30fe81777 Mon Sep 17 00:00:00 2001 From: Marlus Saraiva Date: Sun, 7 Jan 2024 22:31:26 -0300 Subject: [PATCH 3/7] Support debug annotations --- lib/surface.ex | 17 +++++++++++-- lib/surface/compiler/eex_engine.ex | 10 ++++++++ lib/surface/component.ex | 2 +- lib/surface/components/form.ex | 1 + lib/surface/renderer.ex | 12 ++++++++-- test/support/debug_annotations.ex | 23 ++++++++++++++++++ test/support/debug_annotations.sface | 1 + test/surface/compiler_test.exs | 36 +++++++++++++++++++++++++++- 8 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 test/support/debug_annotations.ex create mode 100644 test/support/debug_annotations.sface diff --git a/lib/surface.ex b/lib/surface.ex index ea51e3823..ee21057e9 100644 --- a/lib/surface.ex +++ b/lib/surface.ex @@ -109,6 +109,7 @@ defmodule Surface do indentation = meta[:indentation] || 0 column = meta[:column] || 1 + debug_annotations? = Module.get_attribute(__CALLER__.module, :__debug_annotations__) component_type = Module.get_attribute(__CALLER__.module, :component_type) string @@ -120,7 +121,9 @@ defmodule Surface do |> Surface.Compiler.to_live_struct( debug: Enum.member?(opts, ?d), file: __CALLER__.file, - line: line + line: line, + caller: __CALLER__, + annotate_content: debug_annotations? && (&Phoenix.LiveView.HTMLEngine.annotate_tagged_content/1) ) end @@ -148,11 +151,21 @@ defmodule Surface do if File.exists?(file) do name = file |> Path.rootname() |> Path.basename() + debug_annotations? = + Module.get_attribute( + __CALLER__.module, + :__debug_annotations__, + Application.get_env(:phoenix_live_view, :debug_heex_annotations, false) + ) + body = file |> File.read!() |> Surface.Compiler.compile(1, __CALLER__, file) - |> Surface.Compiler.to_live_struct() + |> Surface.Compiler.to_live_struct( + caller: %Macro.Env{__CALLER__ | file: file, function: {String.to_atom(name), 1}}, + annotate_content: debug_annotations? && (&Phoenix.LiveView.HTMLEngine.annotate_tagged_content/1) + ) quote do @external_resource unquote(file) diff --git a/lib/surface/compiler/eex_engine.ex b/lib/surface/compiler/eex_engine.ex index 3140fa2fc..9afd2709f 100644 --- a/lib/surface/compiler/eex_engine.ex +++ b/lib/surface/compiler/eex_engine.ex @@ -33,6 +33,7 @@ defmodule Surface.Compiler.EExEngine do } nodes + |> maybe_annotate_content(opts[:annotate_content], opts[:caller]) |> to_token_sequence() |> generate_buffer(state.engine.init(opts), state) |> maybe_print_expression( @@ -42,6 +43,15 @@ defmodule Surface.Compiler.EExEngine do ) end + defp maybe_annotate_content(nodes, annotate_content, caller) do + if annotate_content do + {before_comment, after_comment} = annotate_content.(caller) + [%AST.Literal{value: before_comment}] ++ nodes ++ [%AST.Literal{value: after_comment}] + else + nodes + end + end + defp root_tag?(nodes) do Enum.reduce_while(nodes, false, fn %AST.Tag{}, false -> diff --git a/lib/surface/component.ex b/lib/surface/component.ex index cb4f03445..2f5d561d5 100644 --- a/lib/surface/component.ex +++ b/lib/surface/component.ex @@ -39,7 +39,7 @@ defmodule Surface.Component do @before_compile Surface.Renderer @before_compile unquote(__MODULE__) - use Phoenix.Component + use Phoenix.Component, unquote(Keyword.drop(opts, [:slot])) import Phoenix.Component, except: [slot: 1, slot: 2] @behaviour unquote(__MODULE__) diff --git a/lib/surface/components/form.ex b/lib/surface/components/form.ex index e622b7f8c..da75b7bff 100644 --- a/lib/surface/components/form.ex +++ b/lib/surface/components/form.ex @@ -99,6 +99,7 @@ defmodule Surface.Components.Form do assigns = assign(assigns, opts: opts) ~F""" + <.form :let={form} for={@for} action={@action} {...@opts}> <#slot {@default, form: form} context_put={__MODULE__, form: form}/> diff --git a/lib/surface/renderer.ex b/lib/surface/renderer.ex index 1cd486468..e3f86841c 100644 --- a/lib/surface/renderer.ex +++ b/lib/surface/renderer.ex @@ -10,12 +10,20 @@ defmodule Surface.Renderer do template_ast = if File.exists?(template) do - env = Map.put(env, :function, {:render, 1}) + env = + env + |> Map.put(:function, {:render, 1}) + |> Map.put(:file, template) + + debug_annotations? = Module.get_attribute(__CALLER__.module, :__debug_annotations__) template |> File.read!() |> Surface.Compiler.compile(1, env, template) - |> Surface.Compiler.to_live_struct() + |> Surface.Compiler.to_live_struct( + caller: env, + annotate_content: debug_annotations? && (&Phoenix.LiveView.HTMLEngine.annotate_tagged_content/1) + ) else nil end diff --git a/test/support/debug_annotations.ex b/test/support/debug_annotations.ex new file mode 100644 index 000000000..1c239686b --- /dev/null +++ b/test/support/debug_annotations.ex @@ -0,0 +1,23 @@ +defmodule Surface.CompilerTest.DebugAnnotations do + use Surface.Component, debug_heex_annotations: true + + def func_with_text(assigns) do + ~F[func_wiht_text] + end + + def func_with_tag(assigns) do + ~F[
func_with_tag
] + end + + def func_with_text_and_tag(assigns) do + ~F""" + text_before
text_after + """ + end + + def func_with_multiple_tags(assigns) do + ~F""" + text_before
text_after + """ + end +end diff --git a/test/support/debug_annotations.sface b/test/support/debug_annotations.sface new file mode 100644 index 000000000..7e6601ff7 --- /dev/null +++ b/test/support/debug_annotations.sface @@ -0,0 +1 @@ +render \ No newline at end of file diff --git a/test/surface/compiler_test.exs b/test/surface/compiler_test.exs index 3b83aae9d..4b8f73670 100644 --- a/test/surface/compiler_test.exs +++ b/test/surface/compiler_test.exs @@ -1,5 +1,5 @@ defmodule Surface.CompilerTest do - use ExUnit.Case + use Surface.ConnCase, async: true import Surface, only: [sigil_F: 2] import Phoenix.Component, only: [sigil_H: 2] @@ -823,6 +823,40 @@ defmodule Surface.CompilerTest do end) end end + + describe "annotate content" do + test "annotate component with text" do + alias Surface.CompilerTest.DebugAnnotations + import Surface.CompilerTest.DebugAnnotations + + html = + render_surface do + ~F""" +
+ + a + + b + + c + <.func_with_text/> +
+ """ + end + + assert html == """ +
+ render + a + render + b + func_wiht_text + c + func_wiht_text +
+ """ + end + end end defmodule Surface.CompilerSyncTest do From b7cf846bfd0a0c65372c7818f898239c46ccc490 Mon Sep 17 00:00:00 2001 From: Marlus Saraiva Date: Tue, 6 Feb 2024 14:42:39 -0300 Subject: [PATCH 4/7] Fix warnings --- lib/surface/components/form/error_tag.ex | 11 +++++++---- .../compile/surface/validate_components_test.exs | 11 +++++++++-- test/surface/component_test.exs | 11 +++++++++-- test/surface/components/context_test.exs | 12 +++++++++--- test/surface/components/dynamic/component_test.exs | 12 +++--------- .../components/dynamic/live_component_test.exs | 12 +++--------- test/surface/components/events_test.exs | 4 +--- test/surface/components/form/hidden_inputs_test.exs | 4 +--- test/surface/components/form/inputs_test.exs | 4 +--- test/surface/components/link_test.exs | 12 ++++++------ test/surface/integrations/slot_test.exs | 4 +--- 11 files changed, 50 insertions(+), 47 deletions(-) diff --git a/lib/surface/components/form/error_tag.ex b/lib/surface/components/form/error_tag.ex index 762dca711..08ae3faef 100644 --- a/lib/surface/components/form/error_tag.ex +++ b/lib/surface/components/form/error_tag.ex @@ -103,18 +103,21 @@ defmodule Surface.Components.Form.ErrorTag do prop feedback_for, :string def render(assigns) do - translate_error = assigns.translator || translator_from_config() || (&translate_error/1) - class = assigns.class || get_config(:default_class) + assigns = assign(assigns, :class, assigns.class || get_config(:default_class)) ~F""" {translate_error.(error)} + >{translator(assigns.translator).(error)} """ end + defp translator(translator) do + translator || translator_from_config() || (&translate_error/1) + end + @doc """ Translates an error message. diff --git a/test/mix/tasks/compile/surface/validate_components_test.exs b/test/mix/tasks/compile/surface/validate_components_test.exs index 05c2d9c47..e44287c78 100644 --- a/test/mix/tasks/compile/surface/validate_components_test.exs +++ b/test/mix/tasks/compile/surface/validate_components_test.exs @@ -222,11 +222,18 @@ defmodule Mix.Tasks.Compile.Surface.ValidateComponentsTest do use Surface.Component prop list, :list, required: true + data item, :any + data rest, :list def render(%{list: [item | rest]} = assigns) do + assigns = + assigns + |> assign(:item, item) + |> assign(:rest, rest) + ~F""" - {item} - + {@item} + """ end diff --git a/test/surface/component_test.exs b/test/surface/component_test.exs index f0e2fbc24..756073e55 100644 --- a/test/surface/component_test.exs +++ b/test/surface/component_test.exs @@ -144,11 +144,18 @@ defmodule Surface.ComponentTest do prop list, :list prop count, :integer, default: 1 + data item, :any + data rest, :list def render(%{list: [item | rest]} = assigns) do + assigns = + assigns + |> assign(:item, item) + |> assign(:rest, rest) + ~F""" - {@count}. {item} - + {@count}. {@item} + """ end diff --git a/test/surface/components/context_test.exs b/test/surface/components/context_test.exs index 103ddeb3e..0768b0837 100644 --- a/test/surface/components/context_test.exs +++ b/test/surface/components/context_test.exs @@ -850,14 +850,20 @@ defmodule Surface.Components.ContextTest do prop count, :integer, default: 1 data field, :any + data item, :any + data rest, :list def render(%{list: [item | rest]} = assigns) do - assigns = Context.copy_assign(assigns, {ContextTest.Outer, :field}) + assigns = + assigns + |> Context.copy_assign({ContextTest.Outer, :field}) + |> assign(:item, item) + |> assign(:rest, rest) ~F""" - {@count}. {item} - {@field} + {@count}. {@item} - {@field} - + """ end diff --git a/test/surface/components/dynamic/component_test.exs b/test/surface/components/dynamic/component_test.exs index 925a9620c..a2ad689e6 100644 --- a/test/surface/components/dynamic/component_test.exs +++ b/test/surface/components/dynamic/component_test.exs @@ -83,10 +83,8 @@ defmodule Surface.Components.Dynamic.ComponentTest do use Surface.LiveView def render(assigns) do - module = Stateless - ~F""" - + """ end end @@ -95,10 +93,8 @@ defmodule Surface.Components.Dynamic.ComponentTest do use Surface.LiveView def render(assigns) do - module = PhoenixFunctionComponent - ~F""" - + """ end end @@ -163,10 +159,8 @@ defmodule Surface.Components.Dynamic.ComponentTest do use Surface.LiveView def render(assigns) do - module = StatelessWithId - ~F""" - + """ end end diff --git a/test/surface/components/dynamic/live_component_test.exs b/test/surface/components/dynamic/live_component_test.exs index 896880167..755ca94a4 100644 --- a/test/surface/components/dynamic/live_component_test.exs +++ b/test/surface/components/dynamic/live_component_test.exs @@ -74,10 +74,8 @@ defmodule Surface.Components.Dynamic.LiveComponentTest do alias Surface.Components.Dynamic.LiveComponent def render(assigns) do - module = StatefulComponent - ~F""" - + """ end @@ -90,12 +88,10 @@ defmodule Surface.Components.Dynamic.LiveComponentTest do use Surface.LiveView def render(assigns) do - module = StatefulPhoenixLiveComponent - ~F""" """ @@ -107,10 +103,8 @@ defmodule Surface.Components.Dynamic.LiveComponentTest do alias Surface.Components.Dynamic.LiveComponent def render(assigns) do - module = StatefulComponentWithDefaultSlot - ~F""" - + Inner """ diff --git a/test/surface/components/events_test.exs b/test/surface/components/events_test.exs index 30e5785af..d0e6c45c5 100644 --- a/test/surface/components/events_test.exs +++ b/test/surface/components/events_test.exs @@ -8,10 +8,8 @@ defmodule Surface.Components.EventsTest do import Surface.Components.Utils, only: [events_to_opts: 1] def render(assigns) do - attrs = events_to_opts(assigns) - ~F""" -
+
""" end diff --git a/test/surface/components/form/hidden_inputs_test.exs b/test/surface/components/form/hidden_inputs_test.exs index a3db6bbde..bc6b34620 100644 --- a/test/surface/components/form/hidden_inputs_test.exs +++ b/test/surface/components/form/hidden_inputs_test.exs @@ -51,12 +51,10 @@ defmodule Surface.Components.Form.HiddenInputsTest do end test "based on a changeset" do - cs = Parent.changeset(%{children: [%{name: "first"}]}) - html = render_surface do ~F""" -
+ diff --git a/test/surface/components/form/inputs_test.exs b/test/surface/components/form/inputs_test.exs index 0c5985e02..801e37f00 100644 --- a/test/surface/components/form/inputs_test.exs +++ b/test/surface/components/form/inputs_test.exs @@ -31,12 +31,10 @@ defmodule Surface.Components.Form.InputsTest do end test "if the index is received as a slot arg" do - cs = Parent.changeset(%{children: [%{name: "first"}, %{name: "second"}]}) - html = render_surface do ~F""" - +
index: {idx}
diff --git a/test/surface/components/link_test.exs b/test/surface/components/link_test.exs index 11666721b..5ddfc1f6b 100644 --- a/test/surface/components/link_test.exs +++ b/test/surface/components/link_test.exs @@ -136,15 +136,15 @@ defmodule Surface.Components.LinkTest do end test "link with %URI{}" do - url = "https://surface-ui.org/" + assigns = %{url: "https://surface-ui.org/"} - assert render_surface(do: ~F[]) == - render_surface(do: ~F[]) + assert render_surface(do: ~F[]) == + render_surface(do: ~F[]) - path = "/surface" + assigns = %{path: "/surface"} - assert render_surface(do: ~F[]) == - render_surface(do: ~F[]) + assert render_surface(do: ~F[]) == + render_surface(do: ~F[]) end test "link with put/delete" do diff --git a/test/surface/integrations/slot_test.exs b/test/surface/integrations/slot_test.exs index a1f74a593..51b17d01a 100644 --- a/test/surface/integrations/slot_test.exs +++ b/test/surface/integrations/slot_test.exs @@ -264,8 +264,6 @@ defmodule Surface.SlotTest do slot col, as: :cols, arg: %{info: :string}, generator_prop: :items def render(assigns) do - info = "Some info from Grid" - ~F""" @@ -275,7 +273,7 @@ defmodule Surface.SlotTest do
- <#slot {col, info: info} generator_value={item} /> + <#slot {col, info: "Some info from Grid"} generator_value={item} />
From 4b938db7008ceea059b0bf2438a611c7bb55537e Mon Sep 17 00:00:00 2001 From: Marlus Saraiva Date: Wed, 7 Feb 2024 17:58:46 -0300 Subject: [PATCH 5/7] Improve debug annotations --- lib/surface.ex | 39 ++++++----- test/support/debug_annotations.ex | 16 ++--- test/surface/compiler_test.exs | 106 ++++++++++++++++++++++++++---- 3 files changed, 123 insertions(+), 38 deletions(-) diff --git a/lib/surface.ex b/lib/surface.ex index ee21057e9..3983909db 100644 --- a/lib/surface.ex +++ b/lib/surface.ex @@ -151,25 +151,12 @@ defmodule Surface do if File.exists?(file) do name = file |> Path.rootname() |> Path.basename() - debug_annotations? = - Module.get_attribute( - __CALLER__.module, - :__debug_annotations__, - Application.get_env(:phoenix_live_view, :debug_heex_annotations, false) - ) + quote bind_quoted: [file: file, name: name] do + @external_resource file + @file file - body = - file - |> File.read!() - |> Surface.Compiler.compile(1, __CALLER__, file) - |> Surface.Compiler.to_live_struct( - caller: %Macro.Env{__CALLER__ | file: file, function: {String.to_atom(name), 1}}, - annotate_content: debug_annotations? && (&Phoenix.LiveView.HTMLEngine.annotate_tagged_content/1) - ) + body = Surface.__compile_sface__(name, file, __ENV__) - quote do - @external_resource unquote(file) - @file unquote(file) def unquote(String.to_atom(name))(var!(assigns)) do _ = var!(assigns) unquote(body) @@ -185,6 +172,24 @@ defmodule Surface do end end + @doc false + def __compile_sface__(name, file, env) do + debug_annotations? = + Module.get_attribute( + env.module, + :__debug_annotations__, + Application.get_env(:phoenix_live_view, :debug_heex_annotations, false) + ) + + file + |> File.read!() + |> Surface.Compiler.compile(1, env, file) + |> Surface.Compiler.to_live_struct( + caller: %Macro.Env{env | file: file, line: 1, function: {String.to_atom(name), 1}}, + annotate_content: debug_annotations? && (&Phoenix.LiveView.HTMLEngine.annotate_tagged_content/1) + ) + end + @doc """ Converts the given code into Surface's AST. diff --git a/test/support/debug_annotations.ex b/test/support/debug_annotations.ex index 1c239686b..9185fec58 100644 --- a/test/support/debug_annotations.ex +++ b/test/support/debug_annotations.ex @@ -1,23 +1,23 @@ defmodule Surface.CompilerTest.DebugAnnotations do use Surface.Component, debug_heex_annotations: true - def func_with_text(assigns) do - ~F[func_wiht_text] - end - def func_with_tag(assigns) do ~F[
func_with_tag
] end + def func_with_only_text(assigns) do + ~F[only_text] + end + def func_with_text_and_tag(assigns) do ~F""" text_before
text_after """ end - def func_with_multiple_tags(assigns) do - ~F""" - text_before
text_after - """ + def func_with_multiple_root_tags(assigns) do + ~F[
text 1
text 2
] end + + embed_sface "debug_annotations.sface" end diff --git a/test/surface/compiler_test.exs b/test/surface/compiler_test.exs index 4b8f73670..db9f9d4cc 100644 --- a/test/surface/compiler_test.exs +++ b/test/surface/compiler_test.exs @@ -824,22 +824,88 @@ defmodule Surface.CompilerTest do end end - describe "annotate content" do - test "annotate component with text" do - alias Surface.CompilerTest.DebugAnnotations + describe "debug annotations" do + test "show debug info for components with a single root tag" do import Surface.CompilerTest.DebugAnnotations + html = + render_surface do + ~F""" +
+ +
+ """ + end + + assert html == """ +
+
func_with_tag
+
+ """ + end + + test "show debug info for components with multiple root tags" do + html = + render_surface do + ~F""" +
+ +
+ """ + end + + assert html == """ +
+
text 1
text 2
+
+ """ + end + + test "show debug info for components with text only" do + html = + render_surface do + ~F""" +
+ +
+ """ + end + + assert html == """ +
+ only_text +
+ """ + end + + test "show full namespace if the component is imported" do + import Surface.CompilerTest.DebugAnnotations + + html = + render_surface do + ~F""" +
+ <.func_with_tag/> +
+ """ + end + + assert html == """ +
+
func_with_tag
+
+ """ + end + + test "show debug info for module components with a colocated sface file and point to line 1" do + alias Surface.CompilerTest.DebugAnnotations + html = render_surface do ~F"""
- a - b - - c - <.func_with_text/>
""" end @@ -847,12 +913,26 @@ defmodule Surface.CompilerTest do assert html == """
render - a render - b - func_wiht_text - c - func_wiht_text +
+ """ + end + + test "show debug info for component generated by embed_sface/1 and point to line 1" do + import Surface.CompilerTest.DebugAnnotations + + html = + render_surface do + ~F""" +
+ <.debug_annotations/> +
+ """ + end + + assert html == """ +
+ render
""" end From 1cd985acaae08df43848990ef020056296774863 Mon Sep 17 00:00:00 2001 From: Marlus Saraiva Date: Wed, 7 Feb 2024 17:59:01 -0300 Subject: [PATCH 6/7] Fix deprecation warnings --- test/surface/components/live_patch_test.exs | 42 ++++++++----------- .../surface/components/live_redirect_test.exs | 42 ++++++++----------- 2 files changed, 36 insertions(+), 48 deletions(-) diff --git a/test/surface/components/live_patch_test.exs b/test/surface/components/live_patch_test.exs index 4104c484f..9f3fe2153 100644 --- a/test/surface/components/live_patch_test.exs +++ b/test/surface/components/live_patch_test.exs @@ -28,7 +28,9 @@ defmodule Surface.Components.LivePatchTest do """ end - assert html =~ actual_content("user", to: "/users/1") + assert html == """ + user + """ end test "creates a link without label" do @@ -39,7 +41,9 @@ defmodule Surface.Components.LivePatchTest do """ end - assert html =~ actual_content(to: "/users/1") + assert html == """ + + """ end test "creates a link with default slot" do @@ -50,7 +54,9 @@ defmodule Surface.Components.LivePatchTest do """ end - assert html =~ actual_content({:safe, "user"}, to: "/users/1") + assert html == """ + user + """ end test "setting the class" do @@ -61,7 +67,9 @@ defmodule Surface.Components.LivePatchTest do """ end - assert html =~ actual_content("user", to: "/users/1", class: "link") + assert html == """ + user + """ end test "setting multiple classes" do @@ -72,7 +80,9 @@ defmodule Surface.Components.LivePatchTest do """ end - assert html =~ actual_content("user", to: "/users/1", class: "link primary") + assert html == """ + user + """ end test "passing other options" do @@ -88,14 +98,9 @@ defmodule Surface.Components.LivePatchTest do """ end - actual = - actual_content("user", - to: "/users/1", - class: "link", - method: :delete, - data: [confirm: "Really?"], - "csrf-token": "token" - ) + actual = """ + user + """ assert attr_map(html) == attr_map(actual) end @@ -106,15 +111,4 @@ defmodule Surface.Components.LivePatchTest do Map.new(attrs) end - - defp actual_content(text, opts) do - text - |> Phoenix.LiveView.Helpers.live_patch(opts) - |> Phoenix.HTML.html_escape() - |> Phoenix.HTML.safe_to_string() - end - - defp actual_content(opts) do - actual_content("", opts) - end end diff --git a/test/surface/components/live_redirect_test.exs b/test/surface/components/live_redirect_test.exs index e6a756581..c84a12eac 100644 --- a/test/surface/components/live_redirect_test.exs +++ b/test/surface/components/live_redirect_test.exs @@ -27,7 +27,9 @@ defmodule Surface.Components.LiveRedirectTest do """ end - assert html =~ actual_content("user", to: "/users/1") + assert html == """ + user + """ end test "creates a link without label" do @@ -38,7 +40,9 @@ defmodule Surface.Components.LiveRedirectTest do """ end - assert html =~ actual_content(to: "/users/1") + assert html == """ + + """ end test "creates a link with default slot" do @@ -49,7 +53,9 @@ defmodule Surface.Components.LiveRedirectTest do """ end - assert html =~ actual_content({:safe, "user"}, to: "/users/1") + assert html == """ + user + """ end test "setting the class" do @@ -60,7 +66,9 @@ defmodule Surface.Components.LiveRedirectTest do """ end - assert html =~ actual_content("user", to: "/users/1", class: "link") + assert html == """ + user + """ end test "setting multiple classes" do @@ -71,7 +79,9 @@ defmodule Surface.Components.LiveRedirectTest do """ end - assert html =~ actual_content("user", to: "/users/1", class: "link primary") + assert html == """ + user + """ end test "passing other options" do @@ -87,14 +97,9 @@ defmodule Surface.Components.LiveRedirectTest do """ end - actual = - actual_content("user", - to: "/users/1", - class: "link", - method: :delete, - data: [confirm: "Really?"], - "csrf-token": "token" - ) + actual = """ + user + """ assert attr_map(html) == attr_map(actual) end @@ -104,15 +109,4 @@ defmodule Surface.Components.LiveRedirectTest do Map.new(attrs) end - - defp actual_content(text, opts) do - text - |> Phoenix.LiveView.Helpers.live_redirect(opts) - |> Phoenix.HTML.html_escape() - |> Phoenix.HTML.safe_to_string() - end - - defp actual_content(opts) do - actual_content("", opts) - end end From 3f395dec2c6cf91f949ed0edca6227574c495329 Mon Sep 17 00:00:00 2001 From: Marlus Saraiva Date: Wed, 7 Feb 2024 18:36:10 -0300 Subject: [PATCH 7/7] Fix error for unsuported debug_heex_annotations on older phoenix --- test/support/debug_annotations.ex | 4 +++- test/support/debug_annotations_util.ex | 20 ++++++++++++++++++++ test/surface/compiler_test.exs | 10 ++++++---- 3 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 test/support/debug_annotations_util.ex diff --git a/test/support/debug_annotations.ex b/test/support/debug_annotations.ex index 9185fec58..452a1a66b 100644 --- a/test/support/debug_annotations.ex +++ b/test/support/debug_annotations.ex @@ -1,5 +1,7 @@ defmodule Surface.CompilerTest.DebugAnnotations do - use Surface.Component, debug_heex_annotations: true + import Surface.CompilerTest.DebugAnnotationsUtil + + use_component() def func_with_tag(assigns) do ~F[
func_with_tag
] diff --git a/test/support/debug_annotations_util.ex b/test/support/debug_annotations_util.ex new file mode 100644 index 000000000..29359eb77 --- /dev/null +++ b/test/support/debug_annotations_util.ex @@ -0,0 +1,20 @@ +defmodule Surface.CompilerTest.DebugAnnotationsUtil do + def debug_heex_annotations_supported? do + Application.spec(:phoenix_live_view, :vsn) + |> to_string() + |> Version.parse!() + |> Version.compare("0.20.0") != :lt + end + + defmacro use_component() do + if __MODULE__.debug_heex_annotations_supported?() do + quote do + use Surface.Component, debug_heex_annotations: true + end + else + quote do + use Surface.Component + end + end + end +end diff --git a/test/surface/compiler_test.exs b/test/surface/compiler_test.exs index db9f9d4cc..b1e800404 100644 --- a/test/surface/compiler_test.exs +++ b/test/surface/compiler_test.exs @@ -825,6 +825,8 @@ defmodule Surface.CompilerTest do end describe "debug annotations" do + @describetag skip: not Surface.CompilerTest.DebugAnnotationsUtil.debug_heex_annotations_supported?() + test "show debug info for components with a single root tag" do import Surface.CompilerTest.DebugAnnotations @@ -839,7 +841,7 @@ defmodule Surface.CompilerTest do assert html == """
-
func_with_tag
+
func_with_tag
""" end @@ -856,7 +858,7 @@ defmodule Surface.CompilerTest do assert html == """
-
text 1
text 2
+
text 1
text 2
""" end @@ -873,7 +875,7 @@ defmodule Surface.CompilerTest do assert html == """
- only_text + only_text
""" end @@ -892,7 +894,7 @@ defmodule Surface.CompilerTest do assert html == """
-
func_with_tag
+
func_with_tag
""" end