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

updated installation code samples to use Stimulus constant #26

Merged
merged 1 commit into from
May 2, 2024

Conversation

bgartenmann
Copy link
Contributor

in v1.x the application constant was renamed to Stimulus, but this was not yet reflected in the README.

in v1.x the application constant was renamed to Stimulus, but this was not yet reflected in the README.
@tonysm
Copy link
Collaborator

tonysm commented May 2, 2024

Thanks! I'll merge it to reflect the current state, but I'm starting to regret that decision. The Rails folks use application for this (see here). I'm inclined to use that as well and rename the file to be controller/application.js as well. Do you have any thoughts on that @bgartenmann ?

@tonysm tonysm merged commit 668bbab into hotwired-laravel:main May 2, 2024
11 checks passed
@bgartenmann bgartenmann deleted the update-readme branch May 2, 2024 18:42
@bgartenmann
Copy link
Contributor Author

@tonysm What was the reason to change it from application to Stimulus for the 1.x version?
And what makes you regret it now? Any issues with it?
I guess having it the same way as in stimulus-rails is not wrong, however it's mostly a one time set it and forget it, no?

@tonysm
Copy link
Collaborator

tonysm commented May 3, 2024

I wasn't sure if I liked the name application, but now calling it Stimulus is kinda misleading because we have a global Stimulus lib, so the application is really the right name for the application instance (plus, it's the same language they use upstream, so it's a safer bet to avoid confusion, I guess)

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