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

Component generator: warn new users to restart Rails after they create their first component #2047

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iox
Copy link

@iox iox commented Jun 15, 2024

What are you trying to accomplish?

New users can encounter an Uninitialized Constant after they create their first component, as Zeitwerk (current Rails autoloader) will not have app/components in the known paths until the Rails server is restarted. Thanks to @Insti for the help debugging this and @Spone for introducing me to view components in BalticRuby.

This is the expected output:

Screenshot_20240615_075124

What approach did you choose and why?

Our first idea was to use Zeitwerk's push_dir to add the dir to the known folders, but we realized that the running Rails server is completely separate from the generator.

We also thought about making Zeitwerk check for new folders, but it seems like that would clash with their design philosophy.

So we decided to try printing a warning if the folder does not exist and it's about to be created.

Anything you want to highlight for special attention from reviewers?

I'm not sure this is the best place to print this warning, but I couldn't find the exact place where the first app/components file is created.

…d not exist

New users can encounter an UninitializedConstant error after they create their first component, as Zeitwerk will not have "app/components" in the known paths until the Rails server is restarted. Thanks to @Insti for the help debugging this.
@iox iox requested a review from Spone as a code owner June 15, 2024 06:03
@Spone
Copy link
Collaborator

Spone commented Jun 15, 2024

Thanks @iox! I'm wondering if we could call touch tmp/restart.txt from the generator to trigger a server restart.

@joelhawksley
Copy link
Member

@iox what do you think about implementing @Spone's suggestion? I think that would be my preference ❤️

I did look into calling push_dir ahead of time, but you can only call it on directories that exist. So if we wanted to go that route, we'd need to create the app/components directory as part of installing the gem. touch tmp/restart.txt seems like a better idea to me 😄

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