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

Map Stimulus controller filenames to identifiers as in the Stimulus docs #486

Merged
merged 1 commit into from
Jan 24, 2022
Merged

Map Stimulus controller filenames to identifiers as in the Stimulus docs #486

merged 1 commit into from
Jan 24, 2022

Conversation

fpsvogel
Copy link
Contributor

This is a 🐛 bug fix.

I haven't written a test for this change because AFAIK bundled configurations are not included in tests. But let me know if you'd like me to add a test. I did run the tests, though, and none are failing. I also manually tested my changes (more on that below).

Summary

I've made minor changes to the code that the Stimulus bundled configuration adds to index.js, so that Stimulus controllers are detected and named according to the Stimulus docs: https://stimulus.hotwired.dev/handbook/installing#controller-filenames-map-to-identifiers. Specifically, my changes ensure the following:

  • If a Stimulus controller has a name longer than one word, and if the words are separated by underscores in the filename, then in the HTML identifier the words are separated by hyphens and not underscores. E.g. reading_list_controller.js gets the identifier reading-list.
  • If a Stimulus controller has hyphens instead of underscores in the filename, then the controller is still detected and registered as usual, e.g. reading-list-controller.js.

I manually tested my changes by using the changed JS code in my site's index.js. I ensured that it properly detects and registers Stimulus controllers with long multi-word names, whether the filename is separated by underscores or hyphens.

Context

I noticed this discrepancy with the Stimulus docs when I was building a new site in Bridgetown 1.0 beta. The site has a Stimulus controller called reading_list_controller.js, which was being registered as reading_list, which does not match the reading-list identifier in the HTML.

As I was fixing that, I also noticed that the Stimulus doc linked above allows for hyphen-separated Stimulus controller filenames, so I added that into this PR as well.

This is my very first PR to Bridgetown, so let me know if I've missed anything. Thanks!

Copy link
Member

@jaredcwhite jaredcwhite left a comment

Choose a reason for hiding this comment

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

Thanks @fpsvogel, looks good to me!

@jaredcwhite jaredcwhite merged commit c4108f0 into bridgetownrb:main Jan 24, 2022
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.

2 participants