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

Migrate phoenix gen to igniter #261

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

srikanthkyatham
Copy link
Contributor

@srikanthkyatham srikanthkyatham commented Nov 1, 2024

Contributor checklist

Atempt 1 to create PR. Once direction is right. I need to figure out how create tests. Perhaps embed a test project to generate the live views from it.

  • Bug fixes include regression tests
  • Features include unit/acceptance tests

# igniter
Igniter.create_new_file(igniter, destination_path, contents, generate_opts)

# Mix.Generator.create_file(destination_path, contents, generate_opts)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra comments.

@zachdaniel
Copy link
Contributor

This looks great! TO test it, see how we do it in the igniter tests. You'd define a resource in test/support to use as the basis, and then do something like this:

import Igniter.Test

# in your test
test_project()
|> Igniter.compose_task("ash_phoenix.gen.live", [....])
|> assert_creates("....")

One challenge will be a dealing with the mix shell inputs, but there are tools for doing that
https://hexdocs.pm/mix/main/Mix.Shell.Process.html

IIRC you can send the answers to the prompts in order using this kind of code
send(self(), {:mix_shell_input, :prompt, "Pretty cool"})
and then when you run the task it will use those inputs.

One other thing worth noting is that there is an active PR to set up a phoenix project instead of just test_project(), in this PR:

ash-project/igniter#109

We can either wait on that or proceed without it. We can probably proceed without it.

@srikanthkyatham
Copy link
Contributor Author

Cool I will write the tests and make the changes you pointed out. Yeah probably we can continue with out the other PR. Once it is merged I would be happy to contribute back.

@zachdaniel
Copy link
Contributor

Great work, thank you!

@srikanthkyatham
Copy link
Contributor Author

srikanthkyatham commented Nov 3, 2024

@zachdaniel

I figured the Mix shell prompts. Now I am stuck with the test_project setup

    igniter =
      assert test_project(
               files: %{
                 "lib/test/support.ex" => 
                 defmodule Test.Support do
                   use Ash.Domain

                   resources do
                     resource Test.Support.Ticket
                   end
                 end
                 )
                 |> apply_igniter!()
                 |> Igniter.compose_task("ash_phoenix.gen.live", [])

In the task, it is failing at

if !Spark.Dsl.is?(domain, Ash.Domain) do
It looks like I am missing sth. What are those is not quite evident. The tried dbg() the outputs. Not very helpful. Any pointers would be helpful.

@zachdaniel
Copy link
Contributor

Right. So, the difficulty here is that those files are not compiled, and this task uses introspection to determine what to do.

This is a pretty hard problem TBH. I think we could add something like this to igniter (probably in test to start)

def compile(igniter) do
  igniter.rewrite
  |> Stream.map(fn source -> 
     Kernel.ParallelCompiler.async(fn -> 
       Code.compile_string(Rewrite.Source.get(source, :content))
     end)
  end)  
end

@srikanthkyatham
Copy link
Contributor Author

srikanthkyatham commented Nov 4, 2024

Hi @zachdaniel

One other issue, I tried your compile function. It seems that

:erlang.get(:elixir_compiler_info)

is returning undefined. I have not played around Kernel.ParallelCompiler that much. Do you have any insight in this.
Basically the compilation is failing with following stacktrace

 code: |> compile()
     stacktrace:
       (elixir 1.17.3) lib/kernel/parallel_compiler.ex:53: Kernel.ParallelCompiler.inner_async/1
       (elixir 1.17.3) lib/kernel/parallel_compiler.ex:21: Kernel.ParallelCompiler.async/1
       (elixir 1.17.3) lib/stream.ex:613: anonymous fn/4 in Stream.map/2
       (elixir 1.17.3) lib/enum.ex:4858: Enumerable.List.reduce/3
       (elixir 1.17.3) lib/stream.ex:1891: Enumerable.Stream.do_each/4
       (elixir 1.17.3) lib/enum.ex:4423: Enum.reverse/1
       (elixir 1.17.3) lib/enum.ex:3746: Enum.to_list/1
       test/mix/tasks/ash_phoenix.gen.live_test.exs:152: Mix.Tasks.AshPhoenix.Gen.LiveTest.compile/1
       test/mix/tasks/ash_phoenix.gen.live_test.exs:117: (test)

@zachdaniel
Copy link
Contributor

🤔 yeah, so that may not work. I think for now, for these tests, you will have to test with actually defined domains & resources. Like in the test/support folder.

@srikanthkyatham
Copy link
Contributor Author

Hi

Ok I will try to create the resources in the test/support folder and continue with the tests.

@srikanthkyatham
Copy link
Contributor Author

srikanthkyatham commented Nov 5, 2024

Hi @zachdaniel

Almost got it working. The last battle is with the indentation in the assert_creates

Screenshot 2024-11-05 at 22 02 31

I have tried

  1. Mix.Tasks.Format.formatter_for_file
  2. Code.format_string!
  3. String split on the code. I found you have used in one your tests.

The above is blocker now. I have pushed all my changes to the PR.

@srikanthkyatham
Copy link
Contributor Author

Adding LiveView.Formatter helped a bit. Now there is a space at the end of file created

Screenshot 2024-11-05 at 22 39 15

@srikanthkyatham
Copy link
Contributor Author

srikanthkyatham commented Nov 6, 2024

All live gen test pass now. Please check and let me know how we can proceed.

Screenshot 2024-11-06 at 9 44 04

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

Successfully merging this pull request may close these issues.

2 participants