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

Configuration to replace standard url() calls with digested paths #1

Closed
stevestmartin opened this issue Sep 20, 2021 · 12 comments · Fixed by #7
Closed

Configuration to replace standard url() calls with digested paths #1

stevestmartin opened this issue Sep 20, 2021 · 12 comments · Fixed by #7

Comments

@stevestmartin
Copy link

This seems like a great solution to cssbundling-rails#22 and a replacement for Sprockets. However it would be ideal to have the ability to just replace all url() calls with the digested url even if it's behind a configuration flag.

The use cases for this are things like icon fonts, UI libraries, purchased themes all containing CSS that reference assets that come along with them such as images, font files etc.. A conscience effort has to be made (even with Sprockets) to go through and edit the supplied files to use the asset-path helpers. Having the ability to replace all url() calls would allow drop in updates to these types of assets just by dropping the files into folders within config.assets.paths.

Happy to work on a PR if anyone else agrees this is desired.

@ryanb
Copy link

ryanb commented Sep 20, 2021

Wouldn't this conflict with including css through node modules where the assets are not managed by sprockets/propshaft? I think postcss-url or similar is a better fit for managing external assets.

@stevestmartin
Copy link
Author

stevestmartin commented Sep 20, 2021

I don't think there would be any conflict with node_modules related stuff as I would assume as long as the paths were considered during compilation all of those assets would be digested and available to resolve for the replacement. Although I admit i'm far from an expert on this topic.

postcss-url definitely looks like a decent solution, however I guess I was looking for something more simple out of the box. Add a gem, set a feature toggle and call it a day. I suspect many users often download icon fonts, buy themes or download UI libraries / controls (with or without npm/yarn) and aim to just drop them into a predetermined asset folder and have them work. Now I think there is room for a healthy debate on if thats the right or wrong way to go about things, but I think we all can agree the asset pipeline, webpacker, etc.. have all been a sore spot to me in the Rails world for a while, its unnecessarily complex and brittle. A lot of my production related bugs come down to things that look right in development but are broken in production due to sometimes fairly simple issues with configuration of these build pipelines (especially webpacker, especially dev environment in a docker container for some reason).

I think there is definitely room for the let me go setup a build pipeline exactly how I want it, but something about the thought of abstracting away the idea of digesting, and paths changing in production etc..and just going back to the days of i put my assets in folder and use relative paths in them brings a smile to my face.

When I think about the concept its really just a performance optimization, and in my opinion not something we should have to consider when doing development, dropping in theme file updates, or adding additional icons to a font file, etc..

Regardless of if something like this gets implemented, I am extremely happy to see it being an area of focus for Rails 7 with jsbundling, cssbundling, importmaps and hotwired. Asset pipelines are probably the most painful experience of web development and its refreshing to see a move to go back to the basics (unless you require something more complex).

Happy to see your still hanging around the rails community, I miss Rails Casts learned a lot from you in the early 2000's

@brenogazzola
Copy link
Collaborator

Hi @stevestmartin, thank you for already checking Propshaft.

We are still in the investigative stage of the "compilers" feature, but in theory, if we expose the current array as a config option, it should be easy enough for you to copy CssAssetUrls and adjust the regex to pick url instead of asset-path

@stevestmartin
Copy link
Author

I think something like that would be fine. To be clear I'm not opposed to using asset-path just has been the case for sprockets, but it is interesting to think about being able to drop in css and images and not have to modify them with the method call.

@dhh
Copy link
Member

dhh commented Sep 21, 2021

I think this is a great point. Hadn’t even considered it, given how url() can be used for external links too. But clearly we can just deal with the relative ones. Let’s make it so. Don’t need an option either. Should just be url() only.

@ryanb
Copy link

ryanb commented Sep 21, 2021

Would url() parsing use a path relative to the current css file? That seems ideal in order to get theme packs to work out of the box.

This would also work for postcss since it could copy assets to the build directory and doesn't need to add the hash to the file name because propshaft would override the url and generate this.

@dhh
Copy link
Member

dhh commented Sep 21, 2021

It would need to be more clever than what it is now, but we should be able to make that so. The compiler gets the whole asset, so it knows its logical path. There are a few other CSS functions that also take urls, though, and some of them are multiline, so that might be more tricky.

But let's try to proceed with the principle that this is doable with just url, no custom function. @brenogazzola do you want to take a swing?

@brenogazzola
Copy link
Collaborator

@dhh Definitely. @stevestmartin can you point me to some of the UI Libraries, themes, etc. that you would like to use this feature with, so I can test this on real assets?

@stevestmartin
Copy link
Author

stevestmartin commented Sep 21, 2021

@brenogazzola sure, any icon font generator comes to mind as a good example such as Icons8, Nucleo, if your looking for a free alternative Icomoon is a good option.

As far as themes I'm sure any of the bootstrap ones are probably pretty common.

The idea is they generally just come with a css file that uses relative paths to a folder like fonts, images, etc.. so if we could just place these assets within our config.assets.path and the urls self correct that would be awesome.

Granted I'm sure there are bound to be some that use folder naming conventions such asimg ,css ,js that don't match the rails conventions such as this free boostrap theme I just found. But needing to add those to my config.assets.path feels more obvious then go edit all the css that came with this item editing all their paths. This is especially helpful if the theme author provided an update, or you generated a new version of the icon font because you added additional items, you can just drop the file in and not think about it.

@brenogazzola
Copy link
Collaborator

Thanks. I will first get started on all the use cases in the MDN documentation, and after I get something working I'll give it a test in the libraries you suggested:
https://developer.mozilla.org/en-US/docs/Web/CSS/url()

@brenogazzola
Copy link
Collaborator

brenogazzola commented Sep 22, 2021

I've put a draft in case anyone wants to add more use cases for me to handle. A few of the tests are still broken. I'll probably have more time for this later today.

Add digest to valid urls in assets #7

@brenogazzola brenogazzola linked a pull request Sep 23, 2021 that will close this issue
3 tasks
@brenogazzola
Copy link
Collaborator

It's now working for all uses cases I found in MDN and in a real world test. Ready for your review and improvements

@dhh dhh closed this as completed in #7 Oct 9, 2021
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 a pull request may close this issue.

4 participants