Defimpl for some other module stopped working after upgrade to 1.19

Questions / Help
1

Hey guys, I have some files that implement a protocol for some resources for my tests (they are only compiled when running tests:

defimpl Core.Support.Factory.Builder, for: Core.Marketplace.Chat.PropertyMessage do

  def attributes(_message) do
    ...
  end

  def build!(user, _opts) do
    ...
  end
end

Before 1.19, this would work just fine, after the upgrade, I now get this warning:

    warning: you are implementing a protocol for Core.Marketplace.Chat.PropertyMessage but said module is not available. Make sure the module name is correct. If Core.Marketplace.Chat.PropertyMessage is an optional dependency, please wrap the protocol implementation in a Code.ensure_loaded?(Core.Marketplace.Chat.PropertyMessage) check
    │
  1 │ defimpl Core.Support.Factory.Builder, for: Core.Marketplace.Chat.PropertyMessage do
    │ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    │
    └─ test/support/factory/marketplace/chat/property_message.ex:1: (file)

and then the tests that expect this will break.

Did something change in 1.19 related to defimpl or is this a bug?

2

You need to move the implementations to when the protocol is defined, or define them conditionally only for tests.

Once the type system is in place, if we don’t have the protocol definition, it will be impossible to check if the implementation is correct.

3

Do you mean move this code to the same file of the implementation and adding

if Mix.env() == :test do
  defimpl...
end

To it?

Is there some way to declare the dependency on that module before calling defimpl so I don’t have to move test code from test/ to lib/?

4

My understanding is that the protocol is only defined in test, right? So you could also move the implementation to test.

5

The protocol and implementation of it (defimpl) is only in test, but the modelu that the implementation references is in lib.

In other words, the module Core.Marketplace.Chat.PropertyMessage in located in lib/core/marketplace/chat/property_message.ex, the defimpl Core.Support.Factory.Builder, for: Core.Marketplace.Chat.PropertyMessage is located in test/support/factory/marketplace/chat/property_message.ex and the protocol declaration defprotocol Core.Support.Factory.Builder is located in test/support/factory/builder.ex.

Before 1.19, this would work just fine, after 1.19, it started giving the warning/error I mentioned in the first post.

6

This should indeed work. Any chance you can isolate it into a reproducible example so we can address it?

7

Hey @josevalim I made a reproducible example, here is the link: GitHub - sezaru/test_new_elixir

Basically you just need to compile it using MIX_ENV=test mix compile

it will generate the following warning:

> MIX_ENV=test mix compile --force
Compiling 6 files (.ex)
    warning: you are implementing a protocol for Core.Marketplace.Chat.PropertyMessage but said module is not available. Make sure the module name is correct. If Core.Marketplace.Chat.PropertyMessage is an optional dependency, please wrap the protocol implementation in a Code.ensure_loaded?(Core.Marketplace.Chat.PropertyMessage) check
    │
  1 │ defimpl Core.Support.Factory.Builder, for: Core.Marketplace.Chat.PropertyMessage do
    │ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    │
    └─ test/support/factory/marketplace/chat/property_message.ex:1: (file)

Making the example, basically what makes this warning appears or not is the line 11 of the file lib/core/marketplace/chat/property_message.ex:

    attribute :attachments, {:array, Attachment}, default: []

This lines add the embedded Ash resource Attachment as an attribute of my Ash resource PropertyMessage.

If I remove that line, then the warning disappears.

The same code works fine in Elixir 1.18.

Maybe @zachdaniel knows why this triggers the warning?

8

If I remove Ash, then it works. By using –profile time, we can see that Ash is deadlocking the compiler:

$ MIX_ENV=test mix compile --force --profile time
Compiling 6 files (.ex)
[profile]      9ms compiling +      0ms waiting while compiling lib/core/application.ex
[profile]     31ms compiling +      0ms waiting while compiling test/support/factory/builder.ex
[profile]    164ms compiling +      0ms waiting while compiling lib/core/marketplace/chat.ex
[profile] Finished deadlock resolution in 0ms
[profile]     15ms compiling +    133ms waiting for module Core.Marketplace.Chat.PropertyMessage while compiling test/support/factory/marketplace/chat/property_message.ex
[profile]                    |     24ms waiting for module Core.Support.Factory.Builder while compiling test/support/factory/marketplace/chat/property_message.ex
[profile] Finished deadlock resolution in 0ms
[profile]    447ms compiling +    167ms waiting for module :embedded while compiling lib/core/marketplace/chat/attachment.ex
[profile]    239ms compiling +    552ms waiting for module Core.Marketplace.Chat.Attachment while compiling lib/core/marketplace/chat/property_message.ex

The fact that attachment is waiting for :embedded also points to what is more likely a bug. My suggestion is for Ash to move some of the validations in its DSL to verify callbacks. If the compiler is deadlocked, then it assumes the modules are not there, leading to the warning.

1 Like
9

Thanks for finding the root cause @josevalim I will create an issue in Ash’s repo and link to this post.

10

Pretty much all of our validations happen in verifiers, but I will look into this for sure :smiley:

11

This was indeed an issue w/ the :embedded reference there. In Ash, there is a short-hand of data_layer: :embedded. We were treating that as an extension module in spark and attempting to call a function on it. This is fixed in 2.3.7 of spark.

4 Likes
12

I just did a quick test and, indeed, seem to be fixed, thanks!

1 Like