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

Allow detecting Solidus components through a Rails setting #66

Closed
wants to merge 2 commits into from
Closed

Allow detecting Solidus components through a Rails setting #66

wants to merge 2 commits into from

Conversation

waiting-for-dev
Copy link
Contributor

This allows us to define solidus components at the application level, in
contrast to the typical engine definition.

Components paths are loaded on an initializer, which runs before the
application code that could define an Engine class. Adding a setting
on the application.rb, which runs before the initializers, helps us
working around it.

For now, we keep the old detection mechanism, as the other extensions
will need to adapt to it first.

References solidusio/solidus_starter_frontend#162

We were adding the view and controller paths at engine loading time. So,
E.g., if a frontend engine was present, but we loaded it after the
extension depending on `solidus_support` (i.e., the extension went first
on the `Gemfile`), paths weren't being loaded.

We're adding the paths on an initializer that runs before Rails's
[`:initialize_dependency_mechanism`](https://github.com/rails/rails/blob/127dd06df66552dd272eea7832f8bb205cf6fd01/railties/lib/rails/application/bootstrap.rb#L68)
one. Thenceforth Rails begins messing with the load paths, and it doesn't
take them into account anymore.

Keep in mind that the `SolidusSupport.#{engine}_available?` call needs
to be placed within the `initializer` context and not wrapping it. The
reason is the same as above: don't doing the logic at load time.
This allows us to define solidus components at the application level, in
contrast to the typical engine definition.

Components paths are loaded on an initializer, which runs before the
application code that could define an `Engine` class. Adding a setting
on the `application.rb`, which runs before the initializers, helps us
working around it.

For now, we keep the old detection mechanism, as the other extensions
will need to adapt to it first.
@aldesantis
Copy link
Member

I like the actual solution, but I'm opposed to the fundamental idea that we should allow people to run the frontends as a regular Rails engine. I left my thoughts here.

@waiting-for-dev
Copy link
Contributor Author

I like the actual solution, but I'm opposed to the fundamental idea that we should allow people to run the frontends as a regular Rails engine. I left my thoughts here.

Hey @aldesantis. In fact, what this change is allowing is the installation of an engine including SolidusSupport::EngineExtensions (like solidus_auth_devise), into a solidus component present at the application level (like solidus_starter_frontend when it's installed by copying everything) 🙂

@aldesantis
Copy link
Member

@waiting-for-dev yep, that makes sense, although I think solidus_auth_devise should also provide its frontend code in the form of a generator rather than injecting it at runtime. What are your thoughts on that?

@jarednorman
Copy link
Member

I don't know what a transition to that world looks like, but that definitely sounds appealing. Giving users an easy installation method but full control once they've installed everything would be amazing.

@waiting-for-dev
Copy link
Contributor Author

@waiting-for-dev yep, that makes sense, although I think solidus_auth_devise should also provide its frontend code in the form of a generator rather than injecting it at runtime. What are your thoughts on that?

I completely agree, both for the frontend and the backend. I even suggested breaking it into components and including them in the solidus_frontend, solidus_core & solidus_backend gems. However, if that's the way all the extensions are organized, it probably doesn't make sense to bloat the main components. So, yeah, we could keep a couple of generators within the solidus_auth_devise gem.

The question becomes how we build the gem. I think these options may apply to other extensions, too:

  • If the extension doesn't need to hook into the Rails.application, users can download the gem, execute the generators and uninstall the gem.
  • If the extension needs to hook into the Rails.application, users need to add the extension to the Gemfile to run the generators. If the Rails context is only needed during the generation step, they could remove it afterward.
  • We can consider doing the above process automatically from the main components. For instance, a generator on solidus_core could automatically do one of the previous steps.

@aldesantis
Copy link
Member

@waiting-for-dev I think solidus_auth_devise will still need to be installed as a Rails engine because it provides the Spree::User model, and I don't think it would be a good idea to simply copy-paste that into the host app.

For everything else, though, we should just provide a generator.

@waiting-for-dev
Copy link
Contributor Author

I think we can close this one, as it's no longer needed given the direction taken in solidus_starter_frontend (the auth code is also generated). cc @gsmendoza

@waiting-for-dev waiting-for-dev deleted the waiting-for-dev/change_engine_detection branch January 5, 2022 05:12
gsmendoza added a commit to gsmendoza/solidus that referenced this pull request Mar 16, 2022
…are available

Acceptance criteria
-------------------

Given a Solidus extension calls `SolidusSupport.x_available?` in its
Install generator to check if the engine is available

When the I run `bundle exec rake extension:test_app`

Then the `SolidusSupport.x_available?` should return true (since the dummy
app installed by `extension:test_app` includes the frontend, backend, and
api engines)

Bug description
---------------

When called within `bundle exec rake extension:test_app`, the install
generator of the extension is not able to detect the Solidus engines
within the dummy app, and as such, returns nil for any
`SolidusSupport.x_available?` call.

Cause
-----

The `<Extension>::Generators::InstallGenerator.start` call within the
`common:test_app` rake task is not able to detect the Solidus engines that
were just installed by the rake task to the spec/dummy directory.

Bug fix
-------

Use the `bin/rails` executable to install the extension on the dummy app.

Possibly related issues
-----------------------

solidusio/solidus_support#66
gsmendoza added a commit to gsmendoza/solidus that referenced this pull request Mar 16, 2022
…are available

Acceptance criteria
-------------------

Given a Solidus extension calls `SolidusSupport.x_available?` in its
Install generator to check if the engine is available

When the I run `bundle exec rake extension:test_app`

Then the `SolidusSupport.x_available?` should return true (since the dummy
app installed by `extension:test_app` includes the frontend, backend, and
api engines)

Bug description
---------------

When called within `bundle exec rake extension:test_app`, the install
generator of the extension is not able to detect the Solidus engines
within the dummy app, and as such, returns nil for any
`SolidusSupport.x_available?` call.

Cause
-----

The `<Extension>::Generators::InstallGenerator.start` call within the
`common:test_app` rake task is not able to detect the Solidus engines that
were just installed by the rake task to the spec/dummy directory.

Bug fix
-------

Use the `bin/rails` executable to install the extension on the dummy app.

Possibly related issues
-----------------------

solidusio/solidus_support#66
gsmendoza added a commit to gsmendoza/solidus that referenced this pull request Mar 22, 2022
…are available

Acceptance criteria
-------------------

Given a Solidus extension calls `SolidusSupport.x_available?` in its
Install generator to check if the engine is available

When the I run `bundle exec rake extension:test_app`

Then the `SolidusSupport.x_available?` should return true (since the dummy
app installed by `extension:test_app` includes the frontend, backend, and
api engines)

Bug description
---------------

When called within `bundle exec rake extension:test_app`, the install
generator of the extension is not able to detect the Solidus engines
within the dummy app, and as such, returns nil for any
`SolidusSupport.x_available?` call.

Cause
-----

The `<Extension>::Generators::InstallGenerator.start` call within the
`common:test_app` rake task is not able to detect the Solidus engines that
were just installed by the rake task to the spec/dummy directory.

Bug fix
-------

Use the `bin/rails` executable to install the extension on the dummy app.

Possibly related issues
-----------------------

solidusio/solidus_support#66
gsmendoza added a commit to gsmendoza/solidus that referenced this pull request Mar 22, 2022
…are available

Acceptance criteria
-------------------

Given a Solidus extension calls `SolidusSupport.x_available?` in its
Install generator to check if the engine is available

When the I run `bundle exec rake extension:test_app`

Then the `SolidusSupport.x_available?` should return true (since the dummy
app installed by `extension:test_app` includes the frontend, backend, and
api engines)

Bug description
---------------

When called within `bundle exec rake extension:test_app`, the install
generator of the extension is not able to detect the Solidus engines
within the dummy app, and as such, returns nil for any
`SolidusSupport.x_available?` call.

Cause
-----

The `<Extension>::Generators::InstallGenerator.start` call within the
`common:test_app` rake task is not able to detect the Solidus engines that
were just installed by the rake task to the spec/dummy directory.

Bug fix
-------

Use the `bin/rails` executable to install the extension on the dummy app.

Possibly related issues
-----------------------

solidusio/solidus_support#66
rmparr pushed a commit to rmparr/solidus that referenced this pull request Jun 1, 2022
…are available

Acceptance criteria
-------------------

Given a Solidus extension calls `SolidusSupport.x_available?` in its
Install generator to check if the engine is available

When the I run `bundle exec rake extension:test_app`

Then the `SolidusSupport.x_available?` should return true (since the dummy
app installed by `extension:test_app` includes the frontend, backend, and
api engines)

Bug description
---------------

When called within `bundle exec rake extension:test_app`, the install
generator of the extension is not able to detect the Solidus engines
within the dummy app, and as such, returns nil for any
`SolidusSupport.x_available?` call.

Cause
-----

The `<Extension>::Generators::InstallGenerator.start` call within the
`common:test_app` rake task is not able to detect the Solidus engines that
were just installed by the rake task to the spec/dummy directory.

Bug fix
-------

Use the `bin/rails` executable to install the extension on the dummy app.

Possibly related issues
-----------------------

solidusio/solidus_support#66
rmparr pushed a commit to rmparr/solidus that referenced this pull request Jun 1, 2022
…are available

Acceptance criteria
-------------------

Given a Solidus extension calls `SolidusSupport.x_available?` in its
Install generator to check if the engine is available

When the I run `bundle exec rake extension:test_app`

Then the `SolidusSupport.x_available?` should return true (since the dummy
app installed by `extension:test_app` includes the frontend, backend, and
api engines)

Bug description
---------------

When called within `bundle exec rake extension:test_app`, the install
generator of the extension is not able to detect the Solidus engines
within the dummy app, and as such, returns nil for any
`SolidusSupport.x_available?` call.

Cause
-----

The `<Extension>::Generators::InstallGenerator.start` call within the
`common:test_app` rake task is not able to detect the Solidus engines that
were just installed by the rake task to the spec/dummy directory.

Bug fix
-------

Use the `bin/rails` executable to install the extension on the dummy app.

Possibly related issues
-----------------------

solidusio/solidus_support#66
cpfergus1 pushed a commit to cpfergus1/solidus that referenced this pull request Aug 25, 2022
…are available

Acceptance criteria
-------------------

Given a Solidus extension calls `SolidusSupport.x_available?` in its
Install generator to check if the engine is available

When the I run `bundle exec rake extension:test_app`

Then the `SolidusSupport.x_available?` should return true (since the dummy
app installed by `extension:test_app` includes the frontend, backend, and
api engines)

Bug description
---------------

When called within `bundle exec rake extension:test_app`, the install
generator of the extension is not able to detect the Solidus engines
within the dummy app, and as such, returns nil for any
`SolidusSupport.x_available?` call.

Cause
-----

The `<Extension>::Generators::InstallGenerator.start` call within the
`common:test_app` rake task is not able to detect the Solidus engines that
were just installed by the rake task to the spec/dummy directory.

Bug fix
-------

Use the `bin/rails` executable to install the extension on the dummy app.

Possibly related issues
-----------------------

solidusio/solidus_support#66
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.

3 participants