Inconsistent order of attributes when rendering a component in test

antonsatin commented 5 months ago

Environment

Elixir version (elixir -v): Elixir 1.16.0
Phoenix version (mix deps): 1.7.11
Phoenix LiveView version (mix deps): 0.20.2 and main
Operating system: Mac OS Sonoma and GH actions container
Browsers you attempted to reproduce this bug on (the more the merrier): N/A
Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: N/A

Actual behavior

Hi folks 👋🏻 There seems to be an inconsistent order of attributes when rendering components inside templates. It's somehow affected by the --cover flag when running tests. It may also be affected by other things - we discovered it as an intermittent error initially.

@maciej-szlosarczyk isolated the issue to --cover flag and has an example repository with this issue as well as CI check that reproduces this issue.

Given a component like this:

    <div class="flex">
      <.link
        data-foo="something-else"
        class="underline"
        href="https://example.com"
      >
        Some text
      </.link>
    </div>

And a test like this:

  test "should render link component" do
    assigns = %{}

    component_html = """
        <div class=\"flex\">\n  <a href=\"https://example.com\" class=\"underline\" data-foo=\"something-else\">\n    Some text\n  </a>\n</div>
    """

    assert rendered_to_string(~H"""
           <.hello_link />
           """) == String.trim(component_html)
  end

When running the test with and without --cover flag rendered component will produce different results changing order of class and data-foo attributes.

➜ mix test test/hello_web/components/link_component_test.exs
.
Finished in 0.05 seconds (0.05s async, 0.00s sync)
1 test, 0 failures

Randomized with seed 543824

➜ mix test --cover test/hello_web/components/link_component_test.exs
Cover compiling modules ...

  1) test should render link component (HelloWeb.Components.LinkComponentTest)
     test/hello_web/components/link_component_test.exs:9
     Assertion with == failed
     code:  assert rendered_to_string(~H"<.hello_link />
            ") == String.trim(component_html)
     left:  "<div class=\"flex\">\n  <a href=\"https://example.com\" data-foo=\"something-else\" class=\"underline\">\n    Some text\n  </a>\n</div>"
     right: "<div class=\"flex\">\n  <a href=\"https://example.com\" class=\"underline\" data-foo=\"something-else\">\n    Some text\n  </a>\n</div>"
     stacktrace:
       test/hello_web/components/link_component_test.exs:16: (test)

Finished in 0.05 seconds (0.05s async, 0.00s sync)
1 test, 1 failure

I did some debugging and it seems that when assigns arrive to Phoenix.LiveView.TagEngine.component/3 they are in different order.

assigns #=> %{
  __changed__: nil,
  inner_block: [
    %{
      __slot__: :inner_block,
      inner_block: #Function<1.57458588/2 in HelloWeb.Components.MyLinkComponent.hello_link/1>
    }
  ],
  class: "underline",
  "data-foo": "something-else",
  href: "https://example.com"
}

versus

assigns #=> %{
  __changed__: nil,
  inner_block: [
    %{
      __slot__: :inner_block,
      inner_block: #Function<1.96382510/2 in HelloWeb.Components.MyLinkComponent.hello_link/1>
    }
  ],
  "data-foo": "something-else",
  class: "underline",
  href: "https://example.com"
}

That's as far as I could get - I'm not sure where assigns turn from list to a map.

Expected behavior

I would expect order to be consistent like it is between runs without --cover flag, especially since testing docs recommend exact match on strings when testing components.

josevalim commented 5 months ago

Thank you for the report!

assigns are maps and the order of map keys are not guaranteed. It will change based on many factors, it could even be different between mix test executions.

Regardless of the maps behaviour, you should not assert on the generated content, because it leads to brittle tests. Even if order was preserved, <.link could change this order between versions. It would be better to write your tests in a way that you fetch the element and then assert on specific attributes, such as:

content = view |> element("some selector") |> render()
assert content =~ ~s/data-foo="bar"/

Or use Floki alongside Floki.attributes to parse and access attributes in a order independent way. :)

antonsatin commented 5 months ago

Thank you for the swift reply @josevalim!

I agree that with maps it's expected, why I thought it's worth highlighting is because when using html-like syntax it's not obvious that the order may shift. Correct me if I'm wrong, but this concept is also exclusive to components so if we render HEEx with just HTML inside it won't change? So I wanted to clarify if this was an issue with internals or an expected behaviour.

Maybe there's something to add to documentation? Section about components that I linked specifically examples an exact string match when testing components. So perhaps the order nuance can be mentioned in HEEx docs and testings docs. What do you think? Maybe I missed something here :)

josevalim commented 5 months ago

Yes, it is about components because components are just a function. What are you passing is options to a function and there is no guarantee the order may be respected. And, even if it wanted to respect the order, the choice of maps wouldn't help much either. :) Feel free to send a PR for docs if you think there is a particular place it could help. Perhaps some notes on how to assert on the generated results of LiveViewTest.

phoenixframework / phoenix_live_view