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

Any reason why it can't be upstreamed? #6

Open
casperisfine opened this issue Oct 6, 2021 · 2 comments
Open

Any reason why it can't be upstreamed? #6

casperisfine opened this issue Oct 6, 2021 · 2 comments

Comments

@casperisfine
Copy link

I skimmed the code quickly, but didn't notice anything that couldn't really be upstreamed. Is there something I missed?

I see no disadvantage at routes being lazy in development.

@amatsuda
Copy link
Owner

amatsuda commented Oct 6, 2021

Wow, thank you @casperisfine! I'm glad that you liked this.

I skimmed the code quickly, but didn't notice anything that couldn't really be upstreamed. Is there something I missed?

I know that you're too clever that you could skim through this pile of dark matter quickly. But... I confess that I'm already unable to quickly skim this thing that I wrote last year... I mean, the reason why I didn't upstream this is that I just don't want to see such a hack in the Rails codebase.

I see no disadvantage at routes being lazy in development.

Yeah, this is true, in most cases. I thought so too when I started this project, and so the whole patch was a lot more simple and readable, but we found that the real-world apps have exceptions such as rails routes task and workers that render some views (and maybe more), then this monkey-patch collection became bigger and bigger. If we're upstreaming this, maybe we can add a flag to Rails.config that is enabled only for development / test, and with that the patch could be written much simpler. Also, I don't like the current Rack Middleware approach. There should be a better solution than adding a Middleware stack here, but I just chose an easy way that worked.

@gmcgibbon
Copy link

👋 I was looking for ways to speed up boot time in my application recently and stumbled across this gem. I wrote a patch with the same idea, but a slightly different implementation here: rails/rails#51614. If it gets merged, this gem may no longer be needed.

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

No branches or pull requests

3 participants