Remove unused asset generator

[jcoyne]

No description provided.

[jrochkind]

This is removing support for sprockets, or does support for sprockets remain without this generator?

I am not certain if removing support for sprockets has already been decided in a previous PR or community meeting, or if this is the PR that does it?

[jcoyne]

@jrochkind this does nothing to remove sprockets support, but we're not making any effort to generate new applications with sprockets for javascript. Most of the builds (Rails 7.x without propshaft) still use sprockets for css.

[jrochkind]

Ah, thanks, I saw it removing the blacklight:assets:sprockets generator.

My question to understand what's going on here and the contxt, then, I guess, is: Are we removing support for generating with sprockets, or how do we generate with sprockets without this generator?

I thought we were still generating apps with sprockets on Rails 7 in the CI matrix, but was that stopped some time ago? Or is this branch being removed not necessary to generate apps with sprockets in Rails 7?

[jcoyne]

Are we removing support for generating with sprockets, or how do we generate with sprockets without this generator?

All rails 7 installs come with importmap-rails. So this path is never used. Rails 7 still uses sprockets for css.

[jrochkind]

OK, so people with Rails 7 can still do rails new without using importmaps, still using sprockets legacy for asset bundling.

If I understand right, CI has not been testing that path for a while. I hadn't realized that, but ok! I think it still worked.

If I understand right, this PR removes the ability for the blacklight installer to install in these cases, which it previously had before this PR, although CI wasn't testing it.

Is removing support for using Rails 7 without importmaps, with sprockets as JS and CSS bundler, something that has been discusssed at a community meeting? If not, should it be? (These are actual not rhetorical questions, I don't know the answers!)

(Technically, you can still use Rails 8 with sprockets as asset bundler too, but rails new won't generate it for you, which would make it harder to CI certainly).

[jcoyne]

@jrochkind importmap-rails is the default asset delivery mechanism for Rails 7. We have discussed in the community meetings that we must support the default mechanisms. Sprockets is actively discouraged for new apps by the rails core team.

This PR does not remove support for anything. This is just dead code that is being removed. Rails 6 used this I think.

[jrochkind]

OK, the code is not -- if I understand right, I didn't realize until now -- exersized by CI.

While it was not run by CI (since removal of Rails 6 from CI I guess?) -- it is not, if I'm understanding right, "dead code" that can't be run. If I create a Rails 7 app and remove the importmap-rails gem from the Gemfile, and then run the rails g blacklight:install generator -- this code would be used (it is not dead/unreachable code), and I believe it would actually work to set up a BL app as well? (I think it was tested to do so until Rails 6 was removed from CI, recently?)

I feel like we're talking different languages somehow: "We have discussed in the community meetings that we must support the default mechanisms."

Right, my questions is if it's been discussed that we should drop support for legacy sprockets in Rails 7 in addition to supporting importmaps; and if not, if it should be? (It's possible the answer to the second one is that there is no need to discuss it!)

Not if we should support importmaps in Rails 7, I understand that's been discussed. Many many Rails 7 apps (not just BL, in general) use legacy sprockets for JS and CSS, which an out of the box Rails 7 app can do with no need for customization -- so I'm not sure that legacy sprockets is not also a "default mechanism" in Rails 7. But even if it's not considered a default mechanism, deciding to support default mechanisms is not necessarily the same thing as deciding to drop support for anything but default mechanisms.

I could be wrong about any of this stuff -- also even if it is dropping support for something, it's possible that is appropriate, has been discussed, or doesn't need to be discussed, or it could be a quick discussion with agreement! I'm just trying to figure out what's going on and what the situation is with what paths are supported by this app -- both as an attempt to review and for my interest as a developer user -- and I am not really sure why we are having so much trouble communicating.

[jrochkind]

Some of this stuff may now be required by support for npm-based bundling -- but we'll find out if it passes or not on a rebase/merge to main!

[seanaery]

I mentioned in today's Blacklight Community Call that TRLN currently has a shared Blacklight 8-based engine that currently has a generator that does still use the Blacklight sprockets asset generator (in circuitous fashoin), i.e.,

generate 'blacklight:install', '--devise', '--skip-solr', '--skip-assets'
generate 'blacklight:assets:sprockets'

https://github.com/trln/trln_argon/blob/main/spec/test_app_templates/lib/generators/test_app_generator.rb#L18-L34

The institutions using the engine decided that a sprockets-only asset pipeline was still desirable in the short term, hence that approach. So, I would be a bit hesitant about this as a Blacklight 8 change, but I approve it for Blacklight 9.