Test on Rails 8 #3340
Test on Rails 8 #3340
Conversation
508f952 to
a15d1f6
Compare
| return unless defined?(Sprockets) | ||
|
|
There was a problem hiding this comment.
Does this make it impossible to use importmap with propshaft?
There was a problem hiding this comment.
Ah, I think I see – the propshaft-compatible parts are first; everything after this line is sprockets-only?
There was a problem hiding this comment.
If we do this (which I think makes sense) could we also change the conditionals in the asset generator, just to make it explicit that the importmap generator should run regardless of sprockets/propshaft?
There was a problem hiding this comment.
I thought this PR does just that. Maybe you can be more specific and help me understand if it's something else you had in mind.
There was a problem hiding this comment.
My read of the current logic is that only one generator will ultimately run. What I meant is making it possible for two to run: the importmap generator, as well as one of propshaft/sprockets generators. This isn't the case now, because all of the branches are elsif.
There was a problem hiding this comment.
We don't need any other generator to run if the importmap generator runs. The importmap generator handles it. In fact if the importmap generator runs, we must not run the sprockets generator as that sets up the javascript with sprockets.
There was a problem hiding this comment.
What about installing the blacklight-frontend package, which the propshaft generator handles? If we are doing importmaps (for js) and propshaft (for css), don't we still need to ensure that package is installed in order to bring in the scss to compile?
There was a problem hiding this comment.
Yes, the importmap generator handles that.
There was a problem hiding this comment.
Ah, I missed that in the changes in this PR! OK, I think this all makes sense.
No description provided.