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

Creating Umbrella Web App With phx.new.web Results in App That Cannot Be Compiled #5690

Open
kyleboe opened this issue Jan 9, 2024 · 6 comments · May be fixed by #5691
Open

Creating Umbrella Web App With phx.new.web Results in App That Cannot Be Compiled #5690

kyleboe opened this issue Jan 9, 2024 · 6 comments · May be fixed by #5691

Comments

@kyleboe
Copy link

kyleboe commented Jan 9, 2024

Environment

  • Elixir version (elixir -v): 1.16.0
  • Phoenix version (mix deps): 1.7.10
  • Operating system: macOS 14.2.1

Actual Behavior

  1. mix phx.new foo --umbrella
  2. cd foo_umbrella/apps
  3. mix phx.new.web bar
  4. cd ..
  5. mix

There are a couple errors that occur:

  • Related to the appended configuration in config/config.exs:
    config :bar,
      ecto_repos: [Bar.Repo],
      generators: [context_app: false]
    Bar.Repo should not exist in the context of specifically a web application. It is inconsistent with the generated foo_web configuration:
    config :foo_web,
      ecto_repos: [Foo.Repo],
      generators: [context_app: :foo]
  • Related to the generated bar/test/support/conn_case.exs file:
    Bar.DataCase.setup_sandbox(tags)
    Similar to the above, I don't believe this should exist and should instead reference the dependent support module. Also inconsistent with the generated foo_web config:
    Foo.DataCase.setup_sandbox(tags)

Expected Behavior

The app should be able to be compiled when a new web app is generated in the context of an umbrella app.

Proposed Solution

Adding something like a --depends-on flag to the phx.new.web generator so that the backing app can be specified in the generator. The additional context of the flag would also enable adding {:foo, in_umbrella: true}, automatically to deps inside of bar/mix.exs

Example:

mix phx.new.web bar --depends-on foo

Resulting in:

# config/config.exs
config :bar,
  ecto_repos: [Foo.Repo],
  generators: [context_app: :foo]
# bar/test/support/conn_case.exs
Foo.DataCase.setup_sandbox(tags)
# bar/mix.exs
defp deps do
  [
    . . .
    {:foo, in_umbrella: true}
  ]
end
@kyleboe
Copy link
Author

kyleboe commented Jan 9, 2024

I'm happy to open a PR for the proposed solution. Just want to see if there is a more apt name than depends-on or if there is some configuration I am missing.

@kyleboe
Copy link
Author

kyleboe commented Jan 9, 2024

I'm a dummy. That's what the --app flag does.

@kyleboe kyleboe closed this as completed Jan 9, 2024
@kyleboe
Copy link
Author

kyleboe commented Jan 9, 2024

Ok actually I'm not as dumb as I thought (what a sentence).

There are some subtle differences that should be addressed. When passing the --app flag there are other collisions in the generator.

@kyleboe kyleboe reopened this Jan 9, 2024
@chrismccord
Copy link
Member

Please share the details of the collisions you hit. Thanks!

@kyleboe
Copy link
Author

kyleboe commented Jan 10, 2024

Ah yeah sorry I've been working on the PR to fix it. The collisions were in the generated app name. While the parent folder reflects the name passed as the argument (bar in my example) the contents the folder reflect value of the --app flag.

The most clear example of this was in the apps/bar/mix.exs that started with:

defmodule Foo.MixProject do

As well as apps/bar/lib/foo.ex

@kyleboe kyleboe linked a pull request Jan 10, 2024 that will close this issue
@kyleboe
Copy link
Author

kyleboe commented Jan 11, 2024

PR #5691 fixes things in the context of the test suite as well as local manual testing.

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 a pull request may close this issue.

2 participants