Don't create compile-time dependencies to embedded schemas

[crbelaus]

When using polymorphic_embeds_one or polymorphic_embeds_many we create compile-time dependencies from the parent schema to the embedded schemas.

For example, let's say we have the following code:

# lib/article.ex
defmodule Article do
  use Ecto.Schema

  import PolymorphicEmbed

  embedded_schema do
    polymorphic_embeds_many :thing,
      types: [comment: Comment, reaction: Reaction],
      on_replace: :delete
  end
end

# lib/comment.ex
defmodule Comment do
  use Ecto.Schema

  embedded_schema do
    field :content, :string
  end
end

# lib/reaction.ex
defmodule Reaction do
  use Ecto.Schema

  embedded_schema do
    field :content, :string
  end
end

This provides the following output. As we can see, the Article schema has a compile-time dependency on the Comment and Reaction schemas:

mix xref graph --source lib/article.ex --label compile

lib/article.ex
├── lib/comment.ex (compile)
├── lib/polymorphic_embed.ex (compile)
└── lib/reaction.ex (compile)

Ecto had this same issue early on with associations. In particular with embeds_one and embeds_many. They solved it by expanding the aliases and disabling the lexical tracker for associated schemas as shown in elixir-ecto/ecto#1670.

This pull request applies the same solution. This is safe since we don't call any functions in the associated schemas. The result is that running the previous command shows that there are no compile-time dependencies anymore.

mix xref graph --source lib/article.ex --label compile

lib/article.ex
└── lib/polymorphic_embed.ex (compile)

Closes #85

[doughsay]

Hey there, we very much would like to use this library at work, but we use this in our CI:

mix xref graph --label compile-connected --fail-above 0

so any usage of polymorphic embed fails. Would really appreciate it if we could get this PR merged and released! Thanks! ❤️

[mathieuprog]

I merged this on master however there are 2 test failures right now

[mathieuprog]

I actually found out the tests fail because of the mix.lock update, not the expanding of aliases. Specifically:

phoenix_ecto 4.4.3 => 4.5.0

[mathieuprog]

This has been released in 3.0.7. Note that I had to add an additional clause in the case the types are indirectly referenced through a module attribute.

https://github.com/mathieuprog/polymorphic_embed/blob/v.3.0.7/lib/polymorphic_embed.ex#L46

[mathieuprog]

@crbelaus Would you have any opinion on this warning I introduced: https://github.com/mathieuprog/polymorphic_embed/blob/v4.1.0/lib/polymorphic_embed.ex#L42

Firstly is it correct? Secondly is it a good idea?

See also https://github.com/mathieuprog/polymorphic_embed/issues/99

Thank you for any answer

[crbelaus]

Honestly I am not sure @mathieuprog. My understanding of how to handle those compile time dependencies is not that deep.

We may need to ask someone from the Ecto core team and see if they have any insights as they have more experience handling this kind of dependencies (my initial PR was an adaptation of Ecto's own changes).