Add Rails engine support to UrlHelpers compiler - #2632
Conversation
5dab70a to
33f1327
Compare
KaanOzkan
left a comment
KaanOzkan
left a comment
There was a problem hiding this comment.
Implementation looks fine to me but as someone without much context on the compiler it was difficult to wrap my head around it. I have some suggestions on how to improve maintainability. Curious to hear if you have more ideas.
006de5e to
e96f1f5
Compare
|
@KaanOzkan addressed your feedback, PTAL |
KaanOzkan
left a comment
KaanOzkan
left a comment
There was a problem hiding this comment.
Implementation looks good to me, @paracycle are you able to take a look as well? PR is adding significant complexity to add engine support and I want to make sure it's correct.
amomchilov
left a comment
amomchilov
left a comment
There was a problem hiding this comment.
This is a great feature idea, but I'm finding the code kinda hard to follow/review, especially because so many of these concepts have such similar names.
Could you please take a human pass over this and try to see if anything could be simplified? Some documentation on the helper methods would also help
e96f1f5 to
d85f843
Compare
d85f843 to
4c00226
Compare
|
@bdewater-thatch All of my comments were marked resolved, but aren't actually addressed |
Discover Rails engines during URL helper generation and register their engine-scoped path and URL helper modules. Generate mounted helper RBIs for engine mount points, including typed routes proxy classes that include the engine helper modules. Add mounted helper mixins only for classes/modules that actually receive Rails' mounted helpers, and cover the new behavior with regression tests.
4c00226 to
5863f97
Compare
|
@amomchilov my bad, I had an agent rebase and didn't double check the results before re-requesting review. Fixed now. |
|
@bdewater-thatch looks good! that ivar really cleaned up nice. |
3 participants
Motivation
Discover Rails engines during URL helper generation and register their engine-scoped path and URL helper modules. Fixes #474
Generate mounted helper RBIs for engine mount points, including typed routes proxy classes that include the engine helper modules. Add mounted helper mixins only for classes/modules that actually receive Rails' mounted helpers, and cover the new behavior with regression tests.
Implementation
AI assisted with manual refactoring + testing on our own app.