-
-
Notifications
You must be signed in to change notification settings - Fork 292
Fix require.context bug #4004
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
Fix require.context bug #4004
Conversation
Co-authored-by: paul.ionut.bob <[email protected]>
application.register('trix-body', TrixBodyController) | ||
// Special cases for controller registration names | ||
const SPECIAL_CASES = { | ||
nested_form: 'avo-nested-form' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to not make this like the others and eliminate the special cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, since we're extending NestedForm
from 'stimulus-rails-nested-form'
, and the common way to import this controller is as nested-form
, it was causing conflicts with parent apps that also use nested-form
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related PR #3864
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. but what stops us to name it like that?
data-controller="avo-nested-form"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or, change the name that we can change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry could you rephrase? I didn't get your question
I will assume that by "like that" you mean nested-form
.
There are apps using avo
that also import stimulus-rails-nested-form
and register it under the same name: nested-form
. This results in two controllers being registered with the same identifier.
In the app where I encountered and resolved this issue (via the PR linked above), the conflict manifested as duplicate behavior: clicking the "add" button, which was using data-controller="nested-form"
would create two new entries per click, one from our (avo
) controller and one from the parent app's controller.
|
||
// Controllers registry - automatically registers all controllers | ||
const CONTROLLERS = [ | ||
['action', ActionController], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And we then can infer the names.
So in teh end we'd end up with somethign like this
import ActionController from ...
import ...
import ...
registerControllers(application, [Actioncontroller, ..., ...])
it would be easier for us to implement this as well
#2030
Closing this PR since this was made by cursor during some development experimentation that I was doing (can't remeber asking or allowing it but probably i did :D) |
Description
This PR refactors the Stimulus controller registration process to be dynamic and automated, eliminating the need for manual
application.register()
calls.The primary motivation is to simplify controller management, reduce boilerplate, and provide a robust solution that avoids webpack-specific features like
require.context
, ensuring broader compatibility.The changes introduce:
CONTROLLERS
array that lists controller names (snake_case) and their corresponding classes.SPECIAL_CASES
object for custom registration names (e.g.,nested_form
toavo-nested-form
).CONTROLLERS
array, converting names to kebab-case (unless a special case applies) and registering them with the Stimulus application.This approach makes adding new controllers significantly easier (just import and add to the array) and centralizes naming conventions.
Fixes # (issue)
Checklist:
Screenshots & recording
Manual review steps
Manual reviewer: please leave a comment with output from the test if that's the case.
Learn more about Cursor Agents