Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 41 additions & 8 deletions lib/surface/api.ex
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ defmodule Surface.API do
if function_exported?(env.module, :__slots__, 0) do
validate_slot_props_bindings!(env)
end

validate_duplicate_root_props!(env)
end

@doc "Defines a property for the component"
Expand All @@ -106,6 +108,7 @@ defmodule Surface.API do
assign = %{
func: func,
name: name,
as: Keyword.get(opts, :as, name),
type: type,
doc: pop_doc(caller.module),
opts: opts,
Expand All @@ -114,23 +117,48 @@ defmodule Surface.API do
}

assigns = Module.get_attribute(caller.module, :assigns) || %{}
name = Keyword.get(assign.opts, :as, assign.name)
existing_assign = assigns[name]
validate_existing_assign!(assign, assigns, caller)
new_assigns = Map.put(assigns, assign.as, assign)

Module.put_attribute(caller.module, :assigns, new_assigns)
Module.put_attribute(caller.module, assign.func, assign)
end

defp validate_existing_assign!(assign, assigns, caller) do
existing_assign = assigns[assign.as]

if existing_assign do
component_type = Module.get_attribute(caller.module, :component_type)
builtin_assign? = name in Surface.Compiler.Helpers.builtin_assigns_by_type(component_type)

builtin_assign? =
assign.as in Surface.Compiler.Helpers.builtin_assigns_by_type(component_type)

details = existing_assign_details_message(builtin_assign?, existing_assign)
message = ~s(cannot use name "#{assign.name}". #{details}.)

IOHelper.compile_error(message, caller.file, assign.line)
else
assigns = Map.put(assigns, name, assign)
Module.put_attribute(caller.module, :assigns, assigns)
end
end

Module.put_attribute(caller.module, assign.func, assign)
defp validate_duplicate_root_props!(env) do
props =
env.module.__props__()
|> Enum.filter(& &1.opts[:root])

case props do
[prop, _dupicated | _] ->
message = """
cannot define multiple properties as `root: true`. \
Property `#{prop.name}` at line #{prop.line} was already defined as root.

Hint: choose a single property to be the root prop.
"""

IOHelper.compile_error(message, env.file, env.line)

_ ->
nil
end
end

defp existing_assign_details_message(true = _builtin?, %{func: func}) do
Expand Down Expand Up @@ -371,7 +399,7 @@ defmodule Surface.API do
end

defp get_valid_opts(:prop, _type, _opts) do
[:required, :default, :values, :values!, :accumulate]
[:required, :default, :values, :values!, :accumulate, :root]
end

defp get_valid_opts(:data, _type, _opts) do
Expand Down Expand Up @@ -403,6 +431,11 @@ defmodule Surface.API do
value
end

defp validate_opt(:prop, _name, _type, _opts, :root, value, _caller)
when not is_boolean(value) do
{:error, "invalid value for option :root. Expected a boolean, got: #{inspect(value)}"}
end

defp validate_opt(_func, _name, _type, _opts, :required, value, _caller)
when not is_boolean(value) do
{:error, "invalid value for option :required. Expected a boolean, got: #{inspect(value)}"}
Expand Down
50 changes: 38 additions & 12 deletions lib/surface/compiler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,24 @@ defmodule Surface.Compiler do
|> Enum.reverse()
end

defp process_attributes(mod, [{:root, value, attr_meta} | attrs], meta, acc) do
with true <- function_exported?(mod, :__props__, 0),
prop when not is_nil(prop) <- Enum.find(mod.__props__(), & &1.opts[:root]) do
name = Atom.to_string(prop.name)
process_attributes(mod, [{name, value, attr_meta} | attrs], meta, acc)
else
_ ->
message = """
no root property defined for component <#{meta.node_alias}>

Hint: you can declare a root property using option `root: true`
"""

IOHelper.warn(message, meta.caller, fn _ -> attr_meta.line end)
process_attributes(mod, attrs, meta, acc)
end
end

defp process_attributes(mod, [{name, value, attr_meta} | attrs], meta, acc) do
unquoted_string? = attr_meta[:unquoted_string?]
name = String.to_atom(name)
Expand All @@ -613,21 +631,29 @@ defmodule Surface.Compiler do
accumulate? = Keyword.get(type_opts, :accumulate, false)

if not accumulate? and Keyword.has_key?(acc, name) do
IOHelper.warn(
"""
The prop `#{name}` has been passed multiple times. Considering only the last value.
message =
if Keyword.get(type_opts, :root, false) do
"""
the prop `#{name}` has been passed multiple times. Considering only the last value.

Hint: Either specify the `#{name}` via the root property (`<#{meta.node_alias} { ... }>`) or \
explicitly via the #{name} property (`<#{meta.node_alias} #{name}="...">`), but not both.
"""
else
"""
the prop `#{name}` has been passed multiple times. Considering only the last value.

Hint: Either remove all redundant definitions or set option `accumulate` to `true`:

Hint: Either remove all redundant definitions or set option `accumulate` to `true`:
```
prop #{name}, :#{type}, accumulate: true
```

```
prop #{name}, :#{type}, accumulate: true
```
This way the values will be accumulated in a list.
"""
end

This way the values will be accumulated in a list.
""",
meta.caller,
fn _ -> attr_meta.line end
)
IOHelper.warn(message, meta.caller, fn _ -> attr_meta.line end)
end

if unquoted_string? do
Expand Down
25 changes: 24 additions & 1 deletion test/api_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,13 @@ defmodule Surface.APITest do
assert_raise(CompileError, message, fn -> eval(code) end)
end

test "validate :root in prop" do
code = "prop label, :string, root: 1"
message = ~r/invalid value for option :root. Expected a boolean, got: 1/

assert_raise(CompileError, message, fn -> eval(code) end)
end

test "validate duplicate assigns" do
code = """
prop label, :string
Expand Down Expand Up @@ -247,6 +254,22 @@ defmodule Surface.APITest do
{:ok, _module} = eval(code, "LiveView")
end

test "raise compile error for component with multiple root properties" do
code = """
prop title, :string, root: true
prop label, :string, root: true
"""

message = ~r"""
cannot define multiple properties as `root: true`. \
Property `title` at line 4 was already defined as root.

Hint: choose a single property to be the root prop.
"""

assert_raise(CompileError, message, fn -> eval(code) end)
end

test "accept invalid quoted expressions like literal maps as default value" do
code = """
prop map, :map, default: %{a: 1, b: 2}
Expand Down Expand Up @@ -289,7 +312,7 @@ defmodule Surface.APITest do
code = "prop label, :string, a: 1"

message =
~r/unknown option :a. Available options: \[:required, :default, :values, :values!, :accumulate\]/
~r/unknown option :a. Available options: \[:required, :default, :values, :values!, :accumulate, :root\]/

assert_raise(CompileError, message, fn ->
eval(code)
Expand Down
23 changes: 22 additions & 1 deletion test/compiler_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ defmodule Surface.CompilerTest do
defmodule Button do
use Surface.Component

prop label, :string, default: ""
prop label, :string, default: "", root: true
prop click, :event
prop class, :css_class
prop disabled, :boolean
Expand Down Expand Up @@ -205,6 +205,27 @@ defmodule Surface.CompilerTest do
} = node
end

test "component with root property" do
code = """
<Button {"click"} />
"""

[node | _] = Surface.Compiler.compile(code, 1, __ENV__)

assert %Surface.AST.Component{
module: Surface.CompilerTest.Button,
props: [
%Surface.AST.Attribute{
name: :label,
type: :string,
value: %Surface.AST.AttributeExpr{
original: "\"click\""
}
}
]
} = node
end

test "self-closed component with white spaces between attributes" do
code = """
<Button
Expand Down
86 changes: 85 additions & 1 deletion test/properties_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,18 @@ defmodule Surface.PropertiesTest do
end
end

defmodule RootProp do
use Surface.Component

prop label, :string, root: true

def render(assigns) do
~H"""
{ @label }
"""
end
end

describe "string" do
test "passing a string literal" do
assigns = %{text: "text"}
Expand Down Expand Up @@ -596,13 +608,31 @@ defmodule Surface.PropertiesTest do
"""
end
end

describe "root property" do
test "component accepts root property" do
assigns = %{label: "Label"}

html =
render_surface do
~H"""
<RootProp {@label} />
"""
end

assert html =~ """
Label
"""
end
end
end

defmodule Surface.PropertiesSyncTest do
use Surface.ConnCase

import ExUnit.CaptureIO
alias Surface.PropertiesTest.StringProp, warn: false
alias Surface.PropertiesTest.RootProp, warn: false

test "warn if prop is required and has default value" do
id = :erlang.unique_integer([:positive]) |> to_string()
Expand Down Expand Up @@ -658,7 +688,7 @@ defmodule Surface.PropertiesSyncTest do
end)

assert output =~ ~r"""
The prop `label` has been passed multiple times. Considering only the last value.
the prop `label` has been passed multiple times. Considering only the last value.

Hint: Either remove all redundant definitions or set option `accumulate` to `true`:

Expand Down Expand Up @@ -763,4 +793,58 @@ defmodule Surface.PropertiesSyncTest do
prop `invalid_acc2` default value `3` must be a list when `accumulate: true`
"""
end

test "warn if component does not accept a root property" do
assigns = %{label: "root"}

code =
quote do
~H"""
<StringProp
{@label}
/>
"""
end

output =
capture_io(:standard_error, fn ->
compile_surface(code, assigns)
end)

assert output =~ ~r"""
no root property defined for component <StringProp>

Hint: you can declare a root property using option `root: true`

code:2:\
"""
end

test "warn if tag has a root property and the property assigned normally" do
assigns = %{label: "root"}

code =
quote do
~H"""
<RootProp
{@label}
label="toor"
/>
"""
end

output =
capture_io(:standard_error, fn ->
compile_surface(code, assigns)
end)

assert output =~ ~r"""
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.

code:3:\
"""
end
end