Setup generator namespace in generator#6310
Merged
tvdeyen merged 2 commits intosolidusio:mainfrom Aug 5, 2025
Merged
Conversation
A previous solution for this bug tried to set the generator namespace in an app initializer, but that required `rails/generators/base` to be required - which it is not always when starting up e.g. a server. I've tried to dig a bit into how this `mattr_accessor :namespace` on `Rails::Generators::Base` is supposed to be used. It seems like it is run when loading an engine's generators here[1], but that does not work in this context. I don't fully understand how Rails::Generators.namespace is supposed to be set, but this at least sets it only when needed. There's probably some magic that we could invoke, but I can't find it documented anywhere. [1] https://github.com/rails/rails/blob/main/railties/lib/rails/command/actions.rb#L38
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6310 +/- ##
=======================================
Coverage ? 89.31%
=======================================
Files ? 959
Lines ? 20118
Branches ? 0
=======================================
Hits ? 17969
Misses ? 2149
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
adammathys
approved these changes
Jul 31, 2025
tvdeyen
reviewed
Aug 5, 2025
Member
There was a problem hiding this comment.
Read some Rails source code as well and I "think" we can simply move the component generator into the Generators namespace and it "should work"™️
https://github.com/rails/rails/blob/main/railties/lib/rails/generators/base.rb#L56
Can we try that?
Contributor
Author
|
I tried that, it doesn't seem to do the thing. |
Member
Ok, thanks for checking |
tvdeyen
approved these changes
Aug 5, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#6237 introduced setting the generator namespace in an initializer - however, that broke the sandbox and
rails server, because in that initializerRails::Generatorwould only be defined if we're actually running a generator, but not when runningrails serverorrails console.This PR reverts that commit, and instead proposes moving setting the generator namespace to the generator itself. That way, we control a little better what's going on.
I'm not sure this is the best solution out there. My questions are:
SolidusAdmin? Should we assume that all components ever defined for the new admin live in that namespace?Rails::Generators.namespaceusually set? Should we even set it?I do not have answers to these questions. But this PR allows us to run both the component generator and
rails server.Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: