Closed calebhaye closed 1 year ago
This isn't correct, because we want to load the application.js
file from the Stimulus controllers directory. The file is here: https://github.com/hotwired/stimulus-rails/blob/main/lib/install/app/javascript/controllers/application.js
Going up a directory with ..
would include the wrong application.js
file.
The actual solution is that we would need to make sure that stimulus-rails
is installed. That's default in Rails 7 and I don't know if we should even bother supporting older Rails versions? This would alos affect --skip-javascript
apps too I think, but also not sure if we care to support that since we're going to require Javascript for Jumpstart, you know?
This is some example code for checking for duplicates: https://github.com/andyw8/jumpstart/blob/7dd5619f1cfdeb35ba3cefc427b7d1266e7f0173/template.rb#L37
We probably should apply this to all gem dependencies we require, including stimulus-rails
which is assumed right now.
Modifying this, we could have a helper method for gems:
def gemfile_contents
@gemfile_contents ||= IO.read("Gemfile")
end
def add_gem(name, *options)
return if gemfile_contents =~ /^\s*gem ['"]#{name}['"]/
gem name, *options
end
Then we can explicitly include stimulus-rails
and it will just make sure it's there, along with applying this to all the other gems in the list.
@excid3 Thanks for the prompt and thoughtful response. I created and implemented the helper method you suggested, ensured that stimulus-rails
is installed by default, rebased out the prior commit, and added some messaging to reflect that you will need to run bundle
before bin/dev
.
I also changed the post-template-generation instructions to output the db:generate
command after running g notified:model
, because you have to run it after that anyway, so it saves 1 step. I put the message changes in separate commits so they can be pulled out easily if need be -- happy to squash if you agree with them.
With regard to your previous comment, please keep support for Rails 6, at least for now.
I'd also like to see support for svelte in Jumpstart templates one day ... any thoughts on that?
Fixes Issue #176 by changing line of of
app/javascript/controllers/index.js
from:import { application } from "./application"
to
import { application } from "../application"