Various improvements #18

dvic · 2022-01-07T19:25:57Z

This PR contains #15, #16, and #17.

Updates:

Support for modules that call a macro that eventually produces an AST that we known (e.g., an ecto field call). This is an example of such a module:

  defmodule MySchema do
     use TypedEctoSchema

     import MyMacros

     typed_schema "foo" do
       MyMacros.add_field(:foo, :integer)
     end
   end

Support for properly parsing "source" override types, e.g.:

  defmodule MyModule do
     use TypedEctoSchema

     typed_schema "foo" do
       has_many :items, {"some_source", SomeSchema}
     end

   end

Avoids introducing unnecessary compile-time dependencies

This reverts commit 571b1de.

dvic · 2022-01-07T19:26:02Z

I have some additional work todo where I need all three branches merged.

dvic · 2022-01-08T22:22:26Z

@bamorim You have been probably busy and that’s ok :) but do you think you can check this PR anytime soon? I’d really like to get these changes published :) If you need help maintaining the library, let me know, I’m happy to help.

bamorim · 2022-01-08T22:32:40Z

Hey @dvic. I'm a little bit sick now but I'll be taking holidays soon so I'll have time to check this. I'll put it in my calendar so I don't forget.

dvic · 2022-01-08T23:48:54Z

Hope you get better soon! I advertised a bit for the library earlier today ;) https://twitter.com/dvic87/status/1479895011189735425?s=21

lib/typed_ecto_schema/ecto_type_mapper.ex

lib/typed_ecto_schema/syntax_sugar.ex

bamorim · 2022-01-09T15:39:02Z

lib/typed_ecto_schema/syntax_sugar.ex

+    expanded = Macro.expand(unknown, env)
+
+    case expanded do
+      ^unknown ->
+        unknown
+
+      {:__block__, block_context, calls} ->
+        new_calls = Enum.map(calls, &transform_expression(&1, env))
+        {:__block__, block_context, new_calls}
+
+      call ->
+        transform_expression(call, env)
+    end


Ok, so this is the core of this PR, right?
You expand whatever call it is and:

If it's form does not change it means it is not a macro, so you just keep it like that (unknown)

If it expands into a block you get all expressions and transform the expressions recursively

Else it should be just one expression and then you recursively transform that again

Did I got it right?

Also, the test for the block case is the add_to_fields macro, right?

bamorim · 2022-01-09T15:46:54Z

lib/typed_ecto_schema/ecto_type_mapper.ex

+  defp base_type_for({:__aliases__, _, [:Ecto, :Enum]}, opts) do
+    opts
+    |> Keyword.get(:values, [])
+    |> disjunction_typespec()
+  end
+
+  defp base_type_for({:__aliases__, _, _} = ast, _opts) do
+    quote do
+      unquote(ast).t()
+    end
+  end


What this does?
Is it because the type generation was only working if no aliases were being used?
Do we have any testes for that to avoid regression?

Ah, are this what solves the compile deps problem?

Ah, okay, thanks for the explanation via Slack.
So for further reference, this is a required step because now we are calling TypedEctoSchema.TypeBuilder.add_field/5 with a escaped macro to remove the compile time dependency, therefore we don't necessarily have the resolved value and more often than not we have an alias, so this is required to make it work on that scenario while still not requiring a compile time dependency.

Thanks! That is a neat job!

bamorim · 2022-01-09T15:49:30Z

lib/typed_ecto_schema/ecto_type_mapper.ex

-  # Gets the base type for a given Ecto.Type.t()
-  @spec base_type_for(Ecto.Type.t(), field_options()) :: Macro.t()
+  # Gets the base type for a given Ecto.Type.t() or an AST representing a referenced type
+  @spec base_type_for(Ecto.Type.t() | {String.t(), Ecto.Type.t()} | Macro.t(), field_options()) ::


The {String.t(), Ecto.Type.t()} is for the assoc case with custom source, right?
Do you think this type makes sense? Now that I think it is actually a Ecto.Queryable.t() isn't it? (which is sad because it is just a term())

bamorim · 2022-01-09T16:12:40Z

config/config.exs

@@ -1,6 +1,6 @@
 # This file is responsible for configuring your application
 # and its dependencies with the aid of the Mix.Config module.
-use Mix.Config


Do we even need a config? Maybe we can remove config completely.

bamorim · 2022-01-09T16:36:23Z

All right, tested locally and it works.
We still missing (I'll create an issue to track that down and discuss):

Regression tests for the compile time deps
Removing config (we are not using)

randycoulman · 2022-04-07T21:14:26Z

@dvic / @bamorim Thanks for figuring out the circular compile dependencies! We were able to start using this library again because of your work!

bamorim · 2022-04-07T21:50:10Z

It was 100% @dvic. <3

dvic added 17 commits September 4, 2021 12:34

Avoid creating compile-time deps

0751d76

Generate fully qualified AST for embedded types

571b1de

Update typespec

9f3c3cd

Support types represented with 'source override' tuples

938a608

Expand unknown AST calls inside schema block before walking the AST

ed3d6e6

Merge branch 'avoid-compile-time-deps' into fork

e5c3a4f

Merge branch 'schema-with-source-override' into fork

40a5d74

Merge branch 'schema-with-macro-calls' into fork

9bc27bf

Fix credo

95ed225

Merge branch 'schema-with-macro-calls' into fork

0853991

Revert "Generate fully qualified AST for embedded types"

707e274

This reverts commit 571b1de.

Add test for usage of embedded types

2f59a08

Merge branch 'avoid-compile-time-deps' into fork

762e539

Properly handle blocks from expanded macros

e0e49ba

Merge branch 'schema-with-macro-calls'

980a7d1

Handle embeds relations

e86e9c5

Merge branch 'avoid-compile-time-deps' into fork

60b185f

This was referenced Jan 7, 2022

Expand unknown AST calls inside schema block before walking the AST #17

Closed

Support types represented with 'source override' tuples #16

Closed

Avoid creating compile-time deps #15

Closed

Use Config module

5daad60

dvic marked this pull request as ready for review January 8, 2022 22:19

bamorim reviewed Jan 9, 2022

View reviewed changes

lib/typed_ecto_schema/ecto_type_mapper.ex Show resolved Hide resolved

bamorim reviewed Jan 9, 2022

View reviewed changes

lib/typed_ecto_schema/syntax_sugar.ex Show resolved Hide resolved

bamorim reviewed Jan 9, 2022

View reviewed changes

bamorim merged commit 194f4bc into bamorim:master Jan 9, 2022

bamorim mentioned this pull request Jan 9, 2022

Checklist for 0.4.0 #19

Closed

6 tasks

dvic mentioned this pull request Oct 17, 2022

Compile-time dependencies Qqwy/elixir-type_check#162

Open

axelson mentioned this pull request Nov 29, 2022

Fields still cause compile-time dependencies #38

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various improvements #18

Various improvements #18

dvic commented Jan 7, 2022

dvic commented Jan 7, 2022

dvic commented Jan 8, 2022

bamorim commented Jan 8, 2022

dvic commented Jan 8, 2022

bamorim Jan 9, 2022

bamorim Jan 9, 2022

bamorim Jan 9, 2022

bamorim Jan 9, 2022

bamorim Jan 9, 2022

bamorim Jan 9, 2022

bamorim commented Jan 9, 2022

randycoulman commented Apr 7, 2022

bamorim commented Apr 7, 2022

Various improvements #18

Various improvements #18

Conversation

dvic commented Jan 7, 2022

dvic commented Jan 7, 2022

dvic commented Jan 8, 2022

bamorim commented Jan 8, 2022

dvic commented Jan 8, 2022

bamorim Jan 9, 2022

Choose a reason for hiding this comment

bamorim Jan 9, 2022

Choose a reason for hiding this comment

bamorim Jan 9, 2022

Choose a reason for hiding this comment

bamorim Jan 9, 2022

Choose a reason for hiding this comment

bamorim Jan 9, 2022

Choose a reason for hiding this comment

bamorim Jan 9, 2022

Choose a reason for hiding this comment

bamorim commented Jan 9, 2022

randycoulman commented Apr 7, 2022

bamorim commented Apr 7, 2022