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

Properly handles Stimulus controller in parent folders #16

Closed
wants to merge 1 commit into from

Conversation

benoittgt
Copy link

@benoittgt benoittgt commented Apr 25, 2023

If you try to register Stimulus controllers from another path than controller/ the regex does not process it properly.

Also we do not need to add controllers-- for controller in controllers/ folder. See doc.

Examples with dedicated stimulus view components controllers.

import { Application } from "@hotwired/stimulus"

const application = Application.start()
window.Stimulus = application

import componentControllers from '../components/**/*_controller.js';
componentControllers.forEach((controller) => {
  application.register(controller.name, controller.module.default)
});

import controllers from "./**/*_controller.js"
controllers.forEach((controller) => {
  application.register(controller.name, controller.module.default)
})

To try this code.

const ok =[
  'controllers/release_target_controller.js',
  'controllers/admin/toggle_controller.js',
  '../components/deploy_slack_pref_component_controller.js'
].map((module) => module
  .replace(/_controller.[j|t]s$/, "")
  .replace(/^controllers\//, "") // do not namespace controllers in controllers directory
  .replace(/\.\.\//, "") // do not use parent folder anotation for controller name
  .replace(/\//g, "--")
  .replace(/_/g, '-')
)
console.log(ok)

// [
//   'release-target',
//   'admin--toggle',
//   'components--deploy-slack-pref-component'
// ]

This is a breaking change.

Fix: #15

If you try to register Stimulus controllers from another path than
`controller/` the regex do not process it properly.

Also we do not need to add `controllers--` for controller in controller
folder. See [doc](https://stimulus.hotwired.dev/handbook/installing#controller-filenames-map-to-identifiers).

Examples with dedicated stimulus view components controllers.

``js
import { Application } from "@hotwired/stimulus"

const application = Application.start()
window.Stimulus = application

import componentControllers from '../components/**/*_controller.js';
componentControllers.forEach((controller) => {
  application.register(controller.name, controller.module.default)
});

import controllers from "./**/*_controller.js"
controllers.forEach((controller) => {
  application.register(controller.name, controller.module.default)
})
```

To try this code.
```js
const ok =[
 'controllers/release_target_controller.js',
 'controllers/toggle_controller.js',
 '../components/deploy_slack_pref_component_controller.js'
].map((module) => module
            .replace(/_controller.[j|t]s$/, "")
            .replace(/^controllers\//, "") // do not namespace controllers in controllers directory
            .replace(/\.\.\//, "") // do not use parent folder anotation for controller name
            .replace(/\//g, "--")
            .replace(/_/g, '-')
          )
console.log(ok)
// [
//   'release-target',
//   'toggle',
//   'components--deploy-slack-pref-component'
// ]
```

Fix: excid3#15
@excid3
Copy link
Owner

excid3 commented May 1, 2023

I would introduce an index.js in each folder and import them from that directory like Rails does out of the box.

// app/components/index.js
import { application } from "../javascript/controllers/application"

import controllers from "./**/*_controller.js"
controllers.forEach((controller) => {
  application.register(controller.name, controller.module.default)
})
// application.js
import "../components"

We can introduce a config option to strip prefixes in addition, but I don't think it's required.

@excid3 excid3 closed this in 1bcc0f4 Jun 2, 2023
@benoittgt benoittgt deleted the handle-all-controller-path branch June 13, 2023 15:16
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

Successfully merging this pull request may close these issues.

Load Stimulus controller from app/component
2 participants