Discourage SolidusStarterFrontend from being run as an engine #167

gsmendoza · 2021-07-13T09:07:24Z

Dependencies

Depends on #166.

Motivation and Context

From https://www.notion.so/nebulab/Starter-FE-7f00e5d23f5841c1817beebfc8859d83:

We want the new starter frontend to be opinionated and provide a single option which is the "right" way for building your storefront, so we should only allow people to install the starter frontend through a Rails generator and remove (or at the very least severely restrict) the ability to run it as a Rails engine.

Walkthrough and Demo

Demo: https://www.loom.com/share/a7be0f380b944cce9678796e6c88be12

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.

gsmendoza · 2021-07-14T05:57:58Z

docs/development.md

+```sh
+# config/initializers/solidus_starter_frontend.rb
+ENV['SOLIDUS_STARTER_FRONTEND_ALLOW_AS_ENGINE'] = 'true'
+```


I'm not sure if there's a better way to do this. Setting SOLIDUS_STARTER_FRONTEND_ALLOW_AS_ENGINE in the terminal doesn't seem to work with the bundle exec rails generate solidus:install and bundle exec rails generate solidus:auth:install calls in the next step.

gsmendoza · 2021-07-14T05:59:08Z

docs/development.md

@@ -42,6 +52,82 @@ Use Ctrl-C to stop

 Default username and password for admin are: `[email protected]` and `test123`.

+### Running the extension as an engine in a Rails app


As I mentioned in the video, one option is to create a script to automatically run these commands. Let me know what you think!

I think I don't fully understand why we need to provide two different development options. Shouldn't it be enough to have the sandbox load solidus_starter_frontend as a gem? If we point to the local gem (one directory up), changes will also be reflected. WDYT?

@waiting-for-dev The two development options have different goals:

As a gem contributor, I want a easy way to test if a solidus_starter_frontend can generate a working frontend for a Rails app.

As a gem contributor, I want a easy way to preview changes I make to the app code of solidus_starter_frontend.

Previously, the sandbox script was both loading the gem and generating the frontend. In #166, I changed the sandbox script to fulfill only goal 1, based on the reasoning that the gem will be used only as a generator in actual usage.

Goal 2, on the other hand, is more of a nice-to-have for the gem contributor, since it just makes it easier for the contributor to preview changes they make to the app code.

We could change the sandbox script to load the gem instead of using it to generate the frontend. Perhaps I might be misunderstanding what a sandbox app is supposed to do, based on conventions we follow with other Solidus extensions?

Your thoughts?

Hmmm yeah, it makes sense. Ideally, I'd like to confirm that the solidus_starter_frontend executable can generate a working frontend in the integrated test suite. Still, if that's not possible, then I agree we need some way of at least doing it manually.

Perhaps I might be misunderstanding what a sandbox app is supposed to do, based on conventions we follow with other Solidus extensions?

What do you mean by the conventions we follow with other extensions? I haven't worked with Solidus extensions, so I'm unfamiliar. Isn't the sandbox app there picking changes made to the extension code? At least, it's what I do on the main Solidus repo, so I had the idea it was one of its purposes.

Anyway, yeah, probably we need both strategies, but I definitely would automate both of them. Maybe we could even provide a flag to bin/sandbox so switch from one to the other, defaulting to the as-gem version as I think it'll be more common. However, if we take this direction, it'd be nice to be able to configure the application name so that generating it with another strategy doesn't forcefully means overwriting the previous one (we could even use always two different names, like sandbox_as_gem and sandbox_as_exec 🙃 )

I'd like to ping @kennyadsl to hear what are his thoughts on this, too.

@waiting-for-dev I agree with defaulting to as-gem version since 1) this seems to be the more common way you'd want to use the sandbox, and 2) based on what you said, it's how the sandbox behaves in the main Solidus repo.

I also like the idea of have two different scripts/apps for sandbox_as_gem and sandbox_as_exec.

gsmendoza · 2021-07-14T06:00:47Z

lib/solidus_starter_frontend/engine.rb

@@ -15,6 +15,8 @@ class Engine < Rails::Engine
    end

    config.to_prepare do
+      raise 'Error: SolidusStarterFrontend is not allowed to run as an engine' unless ENV['SOLIDUS_STARTER_FRONTEND_ALLOW_AS_ENGINE']


I appreciate your suggestions on improving the copy of this error message.

gsmendoza · 2021-07-14T06:01:17Z

spec/spec_helper.rb

@@ -2,6 +2,7 @@

 # Configure Rails Environment
 ENV['RAILS_ENV'] = 'test'
+ENV['SOLIDUS_STARTER_FRONTEND_ALLOW_AS_ENGINE'] = 'true'


This is needed when running bundle exec rspec.

gsmendoza · 2021-07-14T06:01:39Z

Rakefile

@@ -1,6 +1,8 @@
 # frozen_string_literal: true

 require 'solidus_dev_support/rake_tasks'
+
+ENV['SOLIDUS_STARTER_FRONTEND_ALLOW_AS_ENGINE'] = 'true'


This is needed when running bundle exec rake.

gsmendoza · 2021-07-14T06:20:28Z

docs/development.md

+#### 4) Install the frontend assets
+
+You will need to run `bundle exec rails g solidus_starter_frontend:install` to add the
+frontend assets to the existing vendored Solidus manifest files.


I just realized now that instead of running this install command when running the gem as an engine, we could add the assets directly to app/views/spree/layouts/spree_application.html.erb and then strip them from the layout when running the generator. Let me know what you think!

docs/development.md

gsmendoza · 2021-07-15T04:01:21Z

Temporarily closing until I have a better understanding of #153 and #162.

---- As a solidus_starter_frontend contributor I want the gem's sandbox script to generate the app with the solidus_starter_frontend gem as an engine So that the app would pick up changes I make to the app code Background ---------- Currently, the sandbox script both loads the gem and runs its generator. We want it to only the load the gem. Previous implementations ------------------------ * [Document how to run the extension as an engine in a Rails app](a4600e2) - This commit documents how to install solidus_starter_frontend as an engine. * Fix sandbox script #166 - This PR fixed the script to run `solidus_starter_frontend` as a generator instead of as a gem. Relevant links -------------- #167 (comment) - discussion on having two strategies for generating the sandbox app: one with the Starter FE running as an engine and other where Starter FE is generated into the app. Implementation -------------- * Point solidus gems in sandbox Gemfile to those in solidus_starter_frontend's Gemfile. * Fix: Install solidus using `solidus:install` instead of `spree:install`. * Fix: Pass `--auto-run-migrations` instead of `--auto-accept` to `solidus:auth:install`. * Remove `solidus_starter_frontend` call. * Call `bundle exec rails g solidus_starter_frontend:install`.

Goal ---- As a solidus_starter_frontend contributor I want the gem's sandbox script to generate the app with the solidus_starter_frontend gem as an engine So that the app would pick up changes I make to the app code Background ---------- Currently, the sandbox script both loads the gem and runs its generator. We want it to only the load the gem. Previous implementations ------------------------ * [Document how to run the extension as an engine in a Rails app](a4600e2) - This commit documents how to install solidus_starter_frontend as an engine. * Fix sandbox script #166 - This PR fixed the script to run `solidus_starter_frontend` as a generator instead of as a gem. Relevant links -------------- #167 (comment) - discussion on having two strategies for generating the sandbox app: one with the Starter FE running as an engine and other where Starter FE is generated into the app. Implementation -------------- * Point solidus gems in sandbox Gemfile to those in solidus_starter_frontend's Gemfile. * Fix: Install solidus using `solidus:install` instead of `spree:install`. * Fix: Pass `--auto-run-migrations` instead of `--auto-accept` to `solidus:auth:install`. * Remove `solidus_starter_frontend` call. * Call `bundle exec rails g solidus_starter_frontend:install`.

… app Goal ---- As a `solidus_starter_frontend` contributor I want an alternate sandbox script which copies Starter FE into the sandbox application So that I have an easy way to confirm if the Starter FE can generate a working frontend for a Rails app. Previous implementations ------------------------ * Fix sandbox script #166. Relevant links -------------- #167 (comment) - discussion on having two strategies for generating the sandbox app: one with the Starter FE running as an engine and other where Starter FE is generated into the app.

Goal ---- As a `solidus_starter_frontend` contributor I want an alternate sandbox script which copies Starter FE into the sandbox application So that I have an easy way to confirm if the Starter FE can generate a working frontend for a Rails app. Previous implementations ------------------------ * Fix sandbox script #166. Relevant links -------------- #167 (comment) - discussion on having two strategies for generating the sandbox app: one with the Starter FE running as an engine and other where Starter FE is generated into the app.

gsmendoza added 2 commits July 13, 2021 16:53

Document how to run the extension as an engine in a Rails app

a4600e2

Remove instruction on installing solidus_starter_frontend as a gem

08ad480

gsmendoza force-pushed the gsmendoza/restrict-usage-of-gem-as-engine branch from d632671 to 50b84e3 Compare July 13, 2021 09:15

gsmendoza self-assigned this Jul 13, 2021

gsmendoza changed the title ~~Restrict usage of gem as engine~~ Discourage SolidusStarterFrontend from being ran as an engine Jul 13, 2021

gsmendoza changed the title ~~Discourage SolidusStarterFrontend from being ran as an engine~~ Discourage SolidusStarterFrontend from being run as an engine Jul 13, 2021

gsmendoza force-pushed the gsmendoza/restrict-usage-of-gem-as-engine branch from 50b84e3 to de6a548 Compare July 13, 2021 09:43

Discourage SolidusStarterFrontend from being run as an engine

cfc6371

gsmendoza force-pushed the gsmendoza/restrict-usage-of-gem-as-engine branch from de6a548 to cfc6371 Compare July 13, 2021 10:03

gsmendoza commented Jul 14, 2021

View reviewed changes

gsmendoza marked this pull request as ready for review July 14, 2021 06:02

gsmendoza commented Jul 14, 2021

View reviewed changes

waiting-for-dev changed the base branch from master to fix-sandbox July 14, 2021 09:06

waiting-for-dev reviewed Jul 14, 2021

View reviewed changes

docs/development.md Outdated Show resolved Hide resolved

waiting-for-dev reviewed Jul 14, 2021

View reviewed changes

docs/development.md Outdated Show resolved Hide resolved

Fix heading levels

14893fc

gsmendoza closed this Jul 15, 2021

gsmendoza mentioned this pull request Jul 20, 2021

Fix and update sandbox script #169

Merged

6 tasks

gsmendoza mentioned this pull request Jul 22, 2021

Create alternate sandbox script which copies Starter FE into app #171

Merged

6 tasks

gsmendoza deleted the gsmendoza/restrict-usage-of-gem-as-engine branch November 15, 2022 09:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Discourage SolidusStarterFrontend from being run as an engine #167

Discourage SolidusStarterFrontend from being run as an engine #167

gsmendoza commented Jul 13, 2021 •

edited

Loading

gsmendoza Jul 14, 2021

gsmendoza Jul 14, 2021

waiting-for-dev Jul 14, 2021

gsmendoza Jul 14, 2021

waiting-for-dev Jul 15, 2021

gsmendoza Jul 16, 2021

gsmendoza Jul 14, 2021

gsmendoza Jul 14, 2021

gsmendoza Jul 14, 2021

gsmendoza Jul 14, 2021

gsmendoza commented Jul 15, 2021

		@@ -42,6 +52,82 @@ Use Ctrl-C to stop

		Default username and password for admin are: `[email protected]` and `test123`.

		### Running the extension as an engine in a Rails app

Discourage SolidusStarterFrontend from being run as an engine #167

Discourage SolidusStarterFrontend from being run as an engine #167

Conversation

gsmendoza commented Jul 13, 2021 • edited Loading

Dependencies

Motivation and Context

Walkthrough and Demo

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsmendoza commented Jul 15, 2021

gsmendoza commented Jul 13, 2021 •

edited

Loading