ash icon indicating copy to clipboard operation
ash copied to clipboard

Can no longer call `typed_struct` multiple times

Open ekosz opened this issue 4 months ago • 5 comments

Code of Conduct

  • [x] I agree to follow this project's Code of Conduct

AI Policy

  • [x] I agree to follow this project's AI Policy, or I agree that AI was not used while creating this issue.

Versions

Broke in version v3.5.35 due to this commit https://github.com/ash-project/ash/commit/0fb406aecde970ad8e6252fb82b672ab1bf6f714

Operating system

OSX

Current Behavior

We recently tried upgrading to v3.5.36 from v3.5.34. Once we did we got a new error

(ArgumentError) defstruct has already been called for EKG.Scenes.Events.ChannelFollowed, defstruct can only be called once per module

This module does the following:

defmodule EKG.Scenes.Events.ChannelFollowed do
  use EKG.Scenes.BaseEvent

  typed_struct do
    field :follower_id, :string, allow_nil?: false
    # ... etc ...
  end
end

Where EKG.Scenes.BaseEvent is

defmodule EKG.Scenes.BaseEvent do
  defmacro __using__(_opts) do
    quote do
      typed_struct do
        field :platform, :atom, allow_nil?: false, constraints: [one_of: [:twitch, :youtube]]
        # The "raw" event from original source
        field :raw, :map, allow_nil?: false
      end
    end
  end
end

This idea being that the base event ensured all events had those two fields.

Reproduction

defmodule Test do
  typed_struct do
    field :foo, :string
  end
  typed_struct do
    field :bar, :string
  end
end

### Expected Behavior

Expect the two `typed_struct` calls to be merged as they used to be. OR a user story of how to accomplish what we were doing before

ekosz avatar Aug 27 '25 05:08 ekosz

Hmm...I hadn't anticipated this workflow. There are two options:

  1. add an option to that section like define_struct? that you can set to false and then we skip that logic.
  2. write an extension instead of a __using__ to add the fields you want to add.
defmodule EKG.Scenes.Events.ChannelFollowed do
  use Ash.TypedStruct, extensions: [EKG.Scenes.BaseEvent]

  typed_struct do
    field :follower_id, :string, allow_nil?: false
    # ... etc ...
  end
end

You can see more about writing extensions at https://hexdocs.pm/spark

zachdaniel avatar Aug 28 '25 14:08 zachdaniel

I work with @ekosz. I attempted both options, and failed to get either to work. From what I can tell, most of Spark assumes that all code generation runs at the end of the module definition. Extensions and Transformers then look at the entire state of the DSL at once, and duplicate sections are merged together.

In 0fb406aecde970ad8e6252fb82b672ab1bf6f714 this behavior was intentionally changed to support overriding new on TypedStructs. Spark was given a new after_define feature in d985a38 just for this use-case, which breaks the normal flow and immediately injects code at the end of a section, instead of at the end of the module. Because it runs immediately, Transformers do not get a chance to modify the output, and the after_define hook can not interact with the DSL normally. For instance, it can not get_opt.

Overall, the impression I get is that the addition and usage of after_define goes against the expectations of the rest of the Spark library, and will continue to lead to confusion and issues, especially if usage is increased. I personally would suggest finding a different solution to the new override problem and removing after_define.

For now, I was able to "solve" our issue by reverse-engineering Ash/Spark and finding where the fields are stored, and manually injecting them before typed_struct is called. In the unlikely case that others have this issue, here is the code:

defmodule AshExt.TypedStruct do
  # Reverse-engineered from Ash/Spark source code
  # They store typed_struct fields inside the process
  # https://github.com/ash-project/spark/blob/v2.2.68/lib/spark/dsl/extension.ex#L192
  def add_field(module, %Ash.TypedStruct.Field{} = entity) do
    (Process.get({module, :spark, [:typed_struct]}) || %{entities: [], opts: []})
    |> update_in([:entities], &[entity | &1])
    |> then(&Process.put({module, :spark, [:typed_struct]}, &1))
  end
end

defmodule BaseStruct do
  defmacro __using__(_opts) do
    quote do
      @derive Jason.Encoder
      use Ash.TypedStruct

      AshExt.TypedStruct.add_field(__MODULE__, %Ash.TypedStruct.Field{
        name: :field_one,
        type: :atom,
        allow_nil?: false,
        constraints: [one_of: [:choice_one, :choice_two]]
      })

      AshExt.TypedStruct.add_field(__MODULE__, %Ash.TypedStruct.Field{
        name: :field_two,
        type: :map,
        allow_nil?: false
      })
    end
  end
end

FugiTech avatar Sep 01 '25 06:09 FugiTech

Yes, you're right actually. I'm open to alternative solutions but not being able to override new! which was an explicit part of the API has to take precedence over opening the section twice which is an implicit part of the API.

I'm open to other solutions. An extension wouldn't work, you're right, but adding an option to the section that tells it not to define the module should work I think? You'd have to unset it after checking it though I think.

At the end of the day, you're right and in retrospect that API is problematic, at least to use for this purpose.

zachdaniel avatar Sep 01 '25 12:09 zachdaniel

Did the revert fix this issue?

7d57c728f346491c9b517606e21e157478f95d89

chazwatkins avatar Sep 17 '25 14:09 chazwatkins

Nah, this one is different.

zachdaniel avatar Sep 17 '25 15:09 zachdaniel