-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Deprecate solidus_frontend & allow installing solidus_starter_frontend #4490
Deprecate solidus_frontend & allow installing solidus_starter_frontend #4490
Conversation
8dc8993
to
f43a087
Compare
We need to reflect that solidus_starter_frontend is now installed by default by Solidus. See solidusio/solidus#4490
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is some top-notch work, thanks!
Hi @waiting-for-dev ! I'm encountering the following error when I try to install SolidusFrontend: Here's the Rails app with SolidusFrontend: https://github.com/gsmendoza/rails_7_store/tree/solidus--solidus-4490--solidus-frontend. I didn't encounter any issues with the SolidusStarterFrontend version: https://github.com/gsmendoza/rails_7_store/tree/solidus--solidus-4490--solidus-starter-frontend. I was able to pass the specs and complete a checkout. |
For comparison, here's the Rails app without any frontend: https://github.com/gsmendoza/rails_7_store/tree/solidus--solidus-4490--no-frontend. No issues encountered so far; I was able to start the server on it. |
f43a087
to
92c4055
Compare
Thanks for checking it out, @gsmendoza! That's weird. @kennyadsl also found something similar. The issue comes from the fact that solidus_frontend 3.2.0.alpha can come from two sources: the mono-repo & RubyGems (released from the forked repo). Only the latter contains the install generator. In my tests, that was the one selected by Bundler. However, fetching from I've updated the code so that solidus_frontend is added to the Gemfile with an explicit source pointing to RubyGems. @gsmendoza, could you give it another try? 🙏 |
92c4055
to
871b484
Compare
I also rebased from master after #4494 |
Thanks @waiting-for-dev! I am now able to install Solidus with SolidusFrontend. I used the following Rails app: https://github.com/gsmendoza/rails_7_store/tree/solidus--solidus-4490--solidus-frontend--explicit-source. |
We're recommending new stores use solidus_starter_frontend [1] as their storefront. However, we still want to give the option to install solidus_frontend for now, as some extensions rely on it. We're modifying the Solidus installer to allow installing one or the other. Nonetheless, solidus_frontend is part of the solidus meta-gem [2], which is what is usually bundled in a new installation. At this point, we can't remove solidus_frontend from the meta-gem, as users upgrading from old Solidus versions could see their storefronts suddenly gone. As that will be a breaking change, we're deferring it until v4.0. Until that happens, we must prevent users choosing the new frontend from pulling solidus_frontend. Because of that, we're breaking down the solidus gem into solidus_core, solidus_api, solidus_backend & solidus_sample. We leverage Bundler's injector [3] to add dependencies matching the main solidus gem constraints in the Gemfile. We also use it to remove the meta-gem. Still, we need to extend the upstream injector to support the `path:` option, which is required for local testing (e.g., the sandbox application). We also must break down the meta-gem when solidus_frontend is installed until we remove the frontend directory from this repo. That prevents Bundler from resolving it from there when `path:` or `git:` are used as sources. As per the logic to detect which frontend gets installed, the first of the following conditions met wins: - It's explicitly selected with the `FRONTEND` environment variable (useful for sandbox applications). - It's explicitly selected with the `frontend` option. - If `solidus_frontend` is in the Gemfile, that's the one used. - If the `auto_accept` option is given, `solidus_starter_frontend` is chosen. - The user is prompted to select one or the other. Extensions have been modified to explicitly depend on `solidus_frontend` on their Gemfile, so the dummy app generator used for testing will work OOO matching the third condition. As per its sandbox application, they'll default to `solidus_starter_frontend` because they use `auto_accept`, but they can select `solidus_frontend` through the `FRONTEND` variable. [1] - https://github.com/solidusio/solidus_starter_frontend [2] - https://github.com/solidusio/solidus/blob/a22723079f88387f53f72b503db875b9d97aa3f5/solidus.gemspec#L27 [3] - https://github.com/rubygems/bundler/blob/master/lib/bundler/injector.rb
871b484
to
ebfd2ed
Compare
I also updated the message prompting to install solidus_auth_devise so that users are aware that it'll be installed anyway if they choose solidus_starter_frontend |
We need to reflect that solidus_starter_frontend is now installed by default by Solidus. See solidusio/solidus#4490
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also updated the message prompting to install solidus_auth_devise so that users are aware that it'll be installed anyway if they choose solidus_starter_frontend
It works for now! I think it's acceptable for this transition period.
Thanks @waiting-for-dev, great work as always!
ebfd2ed changed the installer to allow users select solidus_starter_frontend or solidus_frontend as their storefront. When solidus_frontend was added to the Gemfile, the `source:` argument was given to avoid conflicts with the `frontend` directory in the mono-repo. However, the directory was removed in c89fbf0. See solidusio#4490 (comment) for more context
ebfd2ed changed the installer to allow users select solidus_starter_frontend or solidus_frontend as their storefront. If the installer added solidus_frontend to the Gemfile, the `source:` argument was given to avoid conflicts with the `frontend` directory in the mono-repo. However, the directory was removed in c89fbf0. See solidusio#4490 (comment) for more context
We need to reflect that solidus_starter_frontend is now installed by default by Solidus. See solidusio/solidus#4490
We need to reflect that solidus_starter_frontend is now installed by default by Solidus. See solidusio/solidus#4490
Summary
We're recommending new stores use solidus_starter_frontend as their storefront. However, we still want to give the option to install solidus_frontend for now, as some extensions rely on it. We're modifying the Solidus installer to allow installing one or the other.
Nonetheless, solidus_frontend is part of the solidus meta-gem, which is what is usually bundled in a new installation. At this point, we can't remove solidus_frontend from the meta-gem, as users upgrading from old Solidus versions could see their storefronts suddenly gone. As that will be a breaking change, we're deferring it until v4.0.
Until that happens, we must prevent users choosing the new frontend from pulling solidus_frontend. Because of that, we're breaking down the solidus gem into solidus_core, solidus_api, solidus_backend & solidus_sample. We leverage Bundler's injector to add dependencies matching the main solidus gem constraints in the Gemfile. We also use it to remove the meta-gem. Still, we need to extend the upstream injector to support the
path:
option, which is required for local testing (e.g., the sandbox application). We also must break down the meta-gem when solidus_frontend is installed until we remove the frontend directory from this repo. That prevents Bundler from resolving it from there whenpath:
orgit:
are used as sources.As per the logic to detect which frontend gets installed, the first of the following conditions met wins:
FRONTEND
environment variable(useful for sandbox applications).
frontend
option.solidus_frontend
is in the Gemfile, that's the one used.auto_accept
option is given,solidus_starter_frontend
ischosen.
Extensions have been modified to explicitly depend on
solidus_frontend
on their Gemfile, so the dummy app generator used for testing will work OOO matching the third condition. As per its sandbox application, they'll default tosolidus_starter_frontend
because they useauto_accept
, but they can selectsolidus_frontend
through theFRONTEND
variable.Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed (
cross them outif they are not):[ ] I have added automated tests to cover my changes.[ ] I have attached screenshots to demo visual changes.[ ] I have updated the readme to account for my changes.