@joelhawksley @camertron @Spone @boardfish any thought's on this

Sounds to me like the solution is well understood here, so we can go ahead with the described change. @tkowalewski, would you like to open a pull request for this?

@boardfish Yep, I will prepare pull request soon. Thanks

Work in progress (#1951)

You can leave the issue open while working on the PR.

@tkowalewski is their any reason you closed this, because it may still be useful to track the progress of this feature(normally we close the issue once the PR has been merged by adding a closses <issue_num> to the PR description)

Oh, ok

I thought it would be better to have conversations in one place (in the PR)

Thanks for your work on this @tkowalewski – trying to get this working at the moment (but with spec rather than test, if you could bear that in mind) – managed to get the components loading by changing the paths, but struggling with preview paths from within the engine at the moment! Watching with interest... 👀

There's another issue with view_component when it comes to isolated engines - it uses a global configuration. This means that a dummy app can configure config.view_component in one way, and have it tested, but when the engine gets mounted, config.view_component can differ and break expectations.

Right now this is clearly visible with render monkeypatch, preview settings, test controller settings, and url helpers included in view components by default:

• Disabling render monkeypatch can break engines outright

  Engines sticking to render_component avoids the issue.

• Preview doesn't necessarily make sense in the main app, e.g. if it doesn't use view_component

  There's probably a setup where this is outright breaking, but without one it can be postponed for now.

• Global test controller setting enforces use of with_controller_class in engine component tests

  Allow specifying this on a class level that propagates through the inheritance tree.

  E.g.:

  def vc_test_controller
    @vc_test_controller ||= __vc_test_helpers_build_controller(test_controller || Base.test_controller.constantize)
  end

• ViewComponent::Base includes rails application url helpers, forcing use of helpers.something_path, whereas views that are being componentised stick to engine url helpers

  ViewComponent::EngineBase that skips the following could partially deal with the problem:

  # If Rails application is loaded, add application url_helpers to the component context
  # we need to check this to use this gem as a dependency
  if defined?(Rails) && Rails.application && !(child < Rails.application.routes.url_helpers)
    child.include Rails.application.routes.url_helpers
  end

  Subclasses can then include EngineName.instance.routes.url_helpers to have components behave similar to views that are being componentised.

There's another issue with view_component when it comes to isolated engines - it uses a global configuration. This means that a dummy app can configure config.view_component in one way, and have it tested, but when the engine gets mounted, config.view_component can differ and break expectations.

Sound familiar @reeganviljoen? (#1987)

It sounds like we need to consider moving away from global ViewComponent configuration. @boardfish I know you've done a lot of work on our config architecture, what do you think about all of this?

@joelhawksley I am just on holiday now I can take a better look at this next week and let you know.

@joelhawksley I think this is a good idea, but this means we need to make config local to something, so do we make it local to a component, an engine, or anything else?

@reeganviljoen I'm thinking we make it per-component, with the expectation that folks would set config on ApplicationComponent.

Thanks for your work on this @tkowalewski – trying to get this working at the moment (but with spec rather than test, if you could bear that in mind) – managed to get the components loading by changing the paths, but struggling with preview paths from within the engine at the moment! Watching with interest... 👀

@jfi In this pull request (#1951) I am trying to focus on support for generating components in rails engine.
Rails engine is generated with tests by default (you can use flag to disable).
I see issue with support for rspec but for now I don't have time to investigate it more. I will try fix all rspec problems in another pull request.

@joelhawksley Apologies, this thread fell by the wayside! I'd like to take a deeper look into this – off the top of my head, the first thing that comes to mind is how Rails engines get generated with their own internal ApplicationController, ApplicationRecord etc.. This makes me wonder if ViewComponent::Base should have a static default configuration, and all current means of configuring ViewComponent should instead be directed towards a config object that lives on ApplicationComponent?

I'd want to make sure that that's also compatible with engines, but it's harder to make assumptions about where ApplicationComponent is going to live in those.