Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion: refactor as pure AST -> AST transformation #37

Open
bamorim opened this issue Aug 23, 2022 · 0 comments
Open

Discussion: refactor as pure AST -> AST transformation #37

bamorim opened this issue Aug 23, 2022 · 0 comments

Comments

@bamorim
Copy link
Owner

bamorim commented Aug 23, 2022

After the problems we were having with #34 I started thinking if we should change how we generate the types.

Current Approach

The current approach is mostly an "imperative" solution.

typed_schema and typed_embedded_schema macros get the AST and transforms it so that field, timestamps, embeds_one, embeds_many, belongs_to, has_many, has_one and many_to_many translate to something like this

field(:name, :string)
TypeBuilder.add_field(__MODULE__, :field, :name, :string, [])

Then the TypeBuilder.add_field will store type metadata in the module attribute so later TypeBuilder.define_type macro will generate a @type definition.

Proposed Approach

My idea would be to make Typed Ecto Schema more of a simpler AST to AST translation which would generate a AST more similar to how one would write things manually. So that would mean having a pure function that generates the final AST with no calls to any TypedEctoSchema module.

We would even be able to make more isolated tests like

test "generates typecheck code" do
  code = quote do
    field(:str, :string)
  end
  
  type = quote do
    %__MODULE__{
      __meta__:  Ecto.Metadata.t(),
      str: String.t() | nil
    }
  end
  
  assert_equivalent TypeGenerator.type_for(code, :schema, [primary_key: false]), type
end

Another benefit is that this would allow one to "eject" out of TypedEctoSchema if they want to later on by translating the calls on the file (that would require extra tooling, but the core logic would be there).

But mainly, I think after the recursive tree-traversal code is defined, it will be way easier to understand the code as there is less state.

For enforced keys, we would do another traversal getting the enforce: options in the fields completely separated.

I'm still not sure how complicated that is, but I'm starting this discussion to think if this is something worth going for and how we can go about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant