Add Rails engine support to UrlHelpers compiler#2632
Conversation
bdewater-thatch
force-pushed
the
url-helpers-engine-support
branch
from
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.
bdewater-thatch
force-pushed
the
url-helpers-engine-support
branch
2 times, most recently
from
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
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.
bdewater-thatch
force-pushed
the
url-helpers-engine-support
branch
from
e96f1f5 to
d85f843
Compare
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.