improve stimulus controller template to import from correct module #1094
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joelhawksley
approved these changes
Oct 21, 2021
Member
joelhawksley
left a comment
joelhawksley
left a comment
There was a problem hiding this comment.
Excellent! Mind adding a CHANGELOG entry?
Spone
approved these changes
Oct 21, 2021
Stimulus changed the location of their module from `stimulus` to `@hotwired/stimulus`.
Depending on that, the import in the controller template should change.
For `stimulus` it should be
```js
import { Controller } from "stimulus"
```
For `@hotwired/stimulus` it should be
```js
import { Controller } from "@hotwired/stimulus"
```
lxxxvi
force-pushed
the
identify-stimulus-module-for-controller-template
branch
from
43392bb to
0ede388
Compare
Contributor
Author
|
@joelhawksley Thanks for the review! The CHANGELOG entry has been added, feel free to rephrase. |
joelhawksley
approved these changes
Oct 22, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello 👋
Summary
(Addresses #1089)
Stimulus changed the location of their module from
stimulusto@hotwired/stimulus.Depending on that, the import in the stimulus controller template should change.
For
stimulusit should beFor
@hotwired/stimulusit should beThis PR adds this logic and tests.
Other Information
My strategy here was to make
@hotwired/stimulusthe default, unlessstimulusappears in the"dependencies"list inpackage.json. I am not sure if this is best solution or if it should be the other way round?A different approach would be to introduce a new flag to the generator, let's say
--hotwired-stimulus, and modify the template based on that.Any feedback is welcome, thank you!