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

Allow override of default controller #37

Merged
merged 1 commit into from
Sep 13, 2019
Merged

Conversation

hopsoft
Copy link
Contributor

@hopsoft hopsoft commented Sep 13, 2019

No description provided.

@hopsoft hopsoft mentioned this pull request Sep 13, 2019
Copy link
Contributor

@leastbad leastbad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple is best!

@hopsoft hopsoft merged commit f9462f4 into master Sep 13, 2019
@hopsoft hopsoft deleted the hopsoft/default-controller branch September 13, 2019 23:58
@leastbad
Copy link
Contributor

Just a bit of late-breaking thought on an otherwise perfect commit...

I just implemented my first override controller, and it works well, but I had to do some extra bits to get it going. It will have to be documented, is all.

So the basic pattern is that you can/should start off by taking a copy of stimulus_reflex_controller.js from the package/gem and putting it in your app/javascript/controllers folder. I called mine reflex_controller.js.

When I first ran it, I got nasty errors because this.constructor doesn't have a register() method unless you manually set it. You do this for the default controller by assigning the register method manually.

This leaves two options:

The dev can either add 'stimulus_reflex' to their custom controller and change this.constructor.register() to StimulusReflex.register()

or:

The dev can do the manual assignment in their index.js

I'm honestly not sure which one is cleaner. I chose to do it in index.js but mostly to prove to myself that I understood what was going on. It seems like I should convert to importing and call it that way.

However, that brings up the more pertinent question: is there a reason that you didn't just import reflex in the default controller and reference it directly? I don't want to miss anything.

Right now, this is my index.html:

import { Application } from 'stimulus'
import { definitionsFromContext } from 'stimulus/webpack-helpers'
import StimulusReflex from 'stimulus_reflex'
import ReflexController from './reflex_controller'

const application = Application.start()
const context = require.context('controllers', true, /_controller\.js$/)
application.load(definitionsFromContext(context))
ReflexController.register = StimulusReflex.register
StimulusReflex.initialize(application, ReflexController)

It seems like this would keep everything contained to one file that people can copy and modify without having to think about it. It would also allow you to remove the StimulusReflexController.register = register line from stimulus_reflex.js.

@andrewmcodes andrewmcodes added the enhancement New feature or request label Apr 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants