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

The big assets handling / resource transformation funcs naming thread #4854

Closed
bep opened this issue Jun 17, 2018 · 74 comments
Closed

The big assets handling / resource transformation funcs naming thread #4854

bep opened this issue Jun 17, 2018 · 74 comments

Comments

@bep
Copy link
Member

bep commented Jun 17, 2018

This is related to #4429 and #4381 but put in its own thread to get the attention it deserves.

Current Name What is it
resources.Get ✅ Creates a new Resource object given a path to a file in /assets (configurable). This also works for images that you can then scale. Anything with a MIME type recognized by Hugo (and you can define your own if you want).
resources.GetMatch Same as resources.Get, but uses pattern matching to find the first match.
resources.Match Create a slice of Resource objects matching the given pattern. See resources.Concat for a function that could use this.
resources.ToCSS ✅ Transform a Resource to CSS based on the source MIME type. In the first version, we will support SCSS. An implementation note here is that we will persist the result of this transformation, so if you later run this from a non-SASS-enabled Hugo, it will still work. When we finally get some proper plugin support in Hugo, these resource transformations will be candidates to queue up and send to external processors.
resources.PostCSS ✅ See #4858
resources.Minify ✅ Minifies the given Resource based on the source MIME type. Currently supports css, js, json, html, svg, xml. It will get a new RelPermalink and Permalink, e.g. /css/main.min.css.
resources.Fingerprint ✅ Creates a fingerprinted version of the given Resource; it will get a new RelPermalink and Permalink, e.g. /css/main.eae6b4ebececc2bbd7490966a5e01bcc.css. It defaults to sha256, but you can pass either md5, sha256or sha512 as an argument.
Integrity ✅ Available as .Data.Integrity on fingerprinted resources. See https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity. Note that if you want the Integrity, but don't care about the fingerprinting (aka cache-busting part), you can just apply the fingerprint function, and just use .Data.Integrity (i.e. not use RelPermalink etc.)
resources.Concat ✅ Concatenates a list of Resource objects. Think of this as a poor man's bundler.
resources.ExecuteAsTemplate ✅ Parses and executes the given Resource and data context (e.g. .Site) as a Go template. Use your imagination for use cases for this one :-)
resources.FromString ✅ Create a Resource from a string.

All of the above can be chained. So you can do {{ (resources.Resource "styles.scss" | resources.ToCSS | resource.Minify | resources.FingerPrint).RelPermalink }} etc.

Note that I'm not sure that all of the above will arrive in the next Hugo version, but most of these are cheap to implement once the "resource chain" infrastructure is in place.

So I want your input on the above. And note that most of these requires a Resource object to work. You may like or hate that term, but it's there. Which is a reason I have put all of these funcs into the resources namespace.

@bep bep added the Proposal label Jun 17, 2018
@bep bep changed the title The big assets handlin / resources funcs naming thread The big assets handling / resources funcs naming thread Jun 17, 2018
@bep bep changed the title The big assets handling / resources funcs naming thread The big assets handling / resource transformation funcs naming thread Jun 17, 2018
@onedrawingperday
Copy link
Contributor

onedrawingperday commented Jun 17, 2018

This looks mighty cool but how about using a shorter namespace than resources?

Like asset. ( I understand that Hugo will look under /static/ and /content/ for assets but since there will also be a new /assets/ folder why not make the connection between the 2 explicit.)

For example

asset.Resource
asset.ToCSS
asset.Minify
asset.Fingerprint
asset.Concat
asset.Digest
asset.ExecuteTemplate

I also think that resources.Resource might be a bit confusing given that it refers to 2 different Hugo concepts (asset handling & Resource in a Page Bundle).

Just my 2 cents.

@rdwatters
Copy link
Contributor

rdwatters commented Jun 18, 2018

For my own understanding so that I might provide some useful input, is the idea something like:

  1. Until now: “Everything is a Page” (I loved this simplification)
  2. But after this: “Everything is a page; if not, it’s a resource.”

Regardless, I think there is an advantage to continuing to call it resources if it addresses more than just the assets folder. But I might just be confusing a page’s resources vs a page’s assets...

@rdwatters
Copy link
Contributor

@onedrawingperday

I also think that resources.Resource might be a bit confusing

How about asset.Resource in terms of confusion? Why not take what you have above and replace all asset.* with assets.*;e.g., since you don’t concatenate a single asset?

@onedrawingperday
Copy link
Contributor

@rdwatters

Why not take what you have above and replace all asset.* with assets.*;e.g., since you don’t concatenate a single asset?

Sure. I didn't go for the plural because that will be used for the /assets/ folder. Don't know if there might be a conflict. Anyway it's for @bep to decide (I just made a suggestion).

How about asset.Resource in terms of confusion?

Not as confusing as resources.Resource in my eyes.

“Everything is a page; if not, it’s a resource.”

From what I understand it will not be as simple as that. Page Bundle Resources are already Hugo assets ready for processing.

@bep
Copy link
Member Author

bep commented Jun 18, 2018

Until now: “Everything is a Page” (I loved this simplicafication)

So, a Page is also a Resource and can participate in the chain described above. It would not make sense to make "everything a page", and it would be really restrictive if we would stick to that mantra. Resource is an interface with the common behaviour for an item that (may get) rendered to disk. It gets a Permalink. A Page implements the Resource interface. So does Image etc.

I think we should stick to plural to match the other namespaces (collections, partials, strings ec.), so it would become assets.Resource.

Which reads better than resources.Resource -- but we could solve that problem by saying resources.Create or resources.New.

@RealOrangeOne
Copy link
Contributor

I feel like fingerprint and digest should just be available on the object by default? Needing to explicitly add Resource.Fingerprint etc feels very noisy. However, If the intent is to do something magical like automatically generating the related script and link tag, then it's probably fair enough.

@onedrawingperday
Copy link
Contributor

I really like: resources.Create

It reads great and it’s self explanatory.

@TotallyInformation
Copy link

Holy Tintinnabulation Batman :) This is great stuff.

Batman sayings

@bep
Copy link
Member Author

bep commented Jun 18, 2018

I feel like fingerprint and digest should just be available on the object by default?

Not sure what you mean by the object. But we have methods for image resize operations etc. They are very specific to images. But adding all of the above "transformers" as funcs and not methods makes it a little more explicit that this is a chain -- a stream of processes. And it makes it more clear that you create a new instance based on some original.

Also, making it a "cross resource concern" avoids having to walk around an implement it for every Resource.

Given this example:

{{ $css := resources.Create "styles.scss" | resources.ToCSS | resources.Minify | resources.Fingerprint }}
Permalink: {{ $css.Permalink }}
Cotntent: {{ $css.Content }}

Looking at the above it is obvious that you don't want to publish the intermediate steps, you are only interested in the final product. Under the hood this can (and is) implemented very effective in a streaming fashion knowing this.

@bep
Copy link
Member Author

bep commented Jun 18, 2018

@RealOrangeOne note that I have now added resources.Autoprefix to the mix (there was too little work there as-is :-) -- that would probably make sense to have as a method on the CSS resource ... But it kind of belongs somewhere in the middle of a chain, so it would look a little bit funky to have to stop and call a method.

@RealOrangeOne
Copy link
Contributor

@bep sorry, by 'object', i mean the Resource, the thing that comes out the end of the stream. Obviously .Content and .Permalink should be on a Resource by default, but I feel that .Digest should be too. Seeing as it doesn't do anything to the content of the resource, it doesn't feel right to have it in the stream, as in your example above.

I know it's got its own ticket, but IMO AutoPrefix is totally fine to have as an additional stream.

So, what i'm suggesting is more like: (Note the missing resources.Digest stream)

{{ $css := resources.Create "styles.scss" | resources.ToCSS | resources.AutoPrefix | resources.Minify }}
Permalink: {{ $css.Permalink }}
Digest: {{ $css.Digest }}

(Mid way through writing this, i realised .Fingerprint would be mutating the .Permalink to contain a cache-buster, so that probably should be a stream, unless we want to do .FingerprintedPermalink (which I don't really like))

@lucperkins
Copy link

I think that resources.Create doesn't quite fit because you're not creating a styles.scss here. I'd suggest something like resources.Load or resources.Open.

@bep
Copy link
Member Author

bep commented Jun 18, 2018

I think that resources.Create doesn't quite fit because you're not creating a styles.scss here. I'd suggest something like resources.Load or resources.Open.

I agree. Of those 2, I think resources.Open are most accurate. It will always be backed by a file (or something "file like") -- but we will not load the content until really needed, if at all (and we will stream it if possible).

@rdwatters
Copy link
Contributor

rdwatters commented Jun 18, 2018

[[EDIT]]

Removing previous comments once realizing that @regisphilibert's recommendations below make the most sense 😄

@regisphilibert
Copy link
Member

regisphilibert commented Jun 19, 2018

So in a Page Bundle context, current Hugo developers know that .Resources.Get "style.css" will return that resource object ready to be toyed with.

We are used to using Get or GetMatch to fetch resource objects. Could we be using those two functions with this as well ? Instead of .resources.Create, .resources.New, .resources.Resource etc...

Beside .GetMatch can be useful for .Concat

{{ (resources.Get "styles.scss" | resources.ToCSS | resource.Minify | resources.FingerPrint).RelPermalink }}

@bep
Copy link
Member Author

bep commented Jun 19, 2018

@regisphilibert yes, I like using Get -- it feels familiar somehow. I don't think we can easily support .GetMatch though -- we need to find 1 file in a composite filesystem (project, theme etc.), and Afero (the virtual filesystem) does not support pattern matching... It could be done, but not in the first version.

But you are right about the usefulness of .Concat with .GetMatch. I had not thought about that.

@regisphilibert
Copy link
Member

regisphilibert commented Jun 19, 2018

Too bad for GetMatch but I don't think it would have been used this often, .Get is plenty enough.

To get back on .resources.Concat, how will this work as we will have to use it on several resources (without .GetMatch) and define an order of concatenation?

@bep
Copy link
Member Author

bep commented Jun 19, 2018

To get back on .resources.Concat, how will this work as we will have to use it on several resources (without .GetMatch) and define an order of concatenation?

So, the main use case for this (in my head, anyway) is to concatenate resources of the same media type (not sure what would be the end result if you mix and match, but I will have to come to that).

So you would control the order by the argument order to resources.Concat:

{{ $bundle := resources.Concat | slice (resources.Get "first.css") (resources.Get "second.css") }}
{{ $minified := $bundle | resources.Minify }}

Or I guess this should also work:

{{ $bundle := resources.Concat (resources.Get "first.css") (resources.Get "second.css") }}
{{ $minified := $bundle | resources.Minify }}

The bundle variant of the above would be:

{{ $bundle := resources.Concat | resources.Match "*.css" }}
{{ $minified := $bundle | resources.Minify }}

Note that the above is just me jotting on paper. It would be cool with a resources.GetMatch and resources.Match ... I will add them to list above. It should not be hard

@regisphilibert
Copy link
Member

regisphilibert commented Jun 19, 2018

{{ $bundle := resources.Concat | resources.GetMatch "*.css" }}

This would not give control over the order of the files though, which is rather important in most of the case. (I forgot in which order GetMatch returns its matches but if it's alpha, I guess renaming your files so their alpha order matches your concat order would not be too problematic on most projects)

@bep
Copy link
Member Author

bep commented Jun 19, 2018

This would not give control over the order of the files though

Yes it would. The files will be sorted before they are "picked". So you can number your files or whatever.

@bep
Copy link
Member Author

bep commented Jun 19, 2018

That said, I have no ambition to solve all the problems in one go. The important part of this particular issue is the names.

@regisphilibert
Copy link
Member

I seem to have been confusing .GetMatch with .Match in this thread. I meant .Match as a way to get several resources.

@regisphilibert
Copy link
Member

regisphilibert commented Jun 21, 2018

Can we discuss the name of the feature in here as well? How are we going to refer to it in the doc, in the discourse, in the community?

This is about asset management, but a lot of people will instantly understand the impact this will have on their workflow if we identify it also as a task runner (like grunt and gulp). We don't want it to call it what it's not, and it's not a task runner, but something which rings like it maybe...

@bep
Copy link
Member Author

bep commented Jun 21, 2018

Can we discuss the name of the feature in here as well?

Yes, that would be great. But I'm not sure "task runner" is appropriate. I'm starting to get something that is actually working here, and I like this more and more. And what I like most about it (right after the speed) is the simple model of just chaining this into my templates where I use the asset (CSS etc.). I have similar (in concept) with Gulp running as a separate process ... With CSS => static, and then some very elaborate minification fingerprinting process with the renaming of files and string replacement in some template ... And some clever logic to do fingerprinting in production, but not in development ...

{{ $css := resources.Get "styles.scss" }}
{{ if .Site.IsServer }}
{{ $css = $css | resources.ToCSS }}
{{ else }}
{{ $css =  $css  | resources.ToCSS | resources.PostCSS | resources.Minify | resources.Fingerprint }}
{{ end }}

The above is clarity to me. And that template variable assignment is valid in Go 1.11!

There will always people saying that "yes, well, you really need the Asdf NPM JS plugin to umfify your scripts ..." I have said it before and I say it again: I'm ready to sacrifice a lot of flexibility to get the clarity above. And if you look at resources.PostCSS you will not need much imagination to see a "resource plugin" type of transformation step. We just need to play with the above a little first ...

@regisphilibert
Copy link
Member

regisphilibert commented Jun 21, 2018

So Pipeline, Pipes, which gives a sense of running tasks (gulp uses the plumbing semantic a lot) coud be dropped in there... :)

{{ $css := resources.Create "styles.scss" | resources.ToCSS | resources.PostCSS | resources.Minify | resources.Fingerprint }}

Can't stop grinning looking at the above! 😁

@bep
Copy link
Member Author

bep commented Jun 21, 2018

Pipeline, tasks ... Reads to me a something "running a set of tasks/jobs/script", "running as a batch", i.e. something that is declared in a build config and is ... not fast. But I don't have a good suggestion for an alternative.

@bep
Copy link
Member Author

bep commented Jun 28, 2018

I have renamed one and added one in the table above:

resources.FromString and resources.FromTemplate, and yes, both will be in Hugo 0.43.


@regisphilibert
Copy link
Member

I am missing something about.resources.FromString. How is it different than .resources.Get?

@bep
Copy link
Member Author

bep commented Jun 28, 2018

How is it different than .resources.Get

Get takes a filename which is, of course, also a string, but conceptually different.

@regisphilibert
Copy link
Member

regisphilibert commented Jun 28, 2018

Thanks.
Quick follow up question 🖐.
This is what I make of .Get and .FromString:

  • resources.Get "customVariables.scss"
  • resources.FromString (printf "$headerColor:%s" .Params.headerColor)

Now, I'm having a hard trim grasping the concept of .FromTemplate, what kind of argument does it expect?

@bep
Copy link
Member Author

bep commented Jun 28, 2018

Now, I'm having a hard trim grasping the concept of .FromTemplate, what kind of argument does it expect?

I was going to say "wait for the documentation", but since you're the one who be writing that (:-)):

  • FromString and FromTemplate are on the API-side very similar.
{{ $r := "Hugo Rocks!" | resources.FromString "rocks/hugo.txt" }}

You will typically pipe in the content from some source ... But reading the above, I think we can improve and make the resource.FromTemplate something that takes a resource. I will think a little.

@regisphilibert
Copy link
Member

I am closer to getting it, I'm curious to find out the content of rocks/hugo.txt now :) This I can find out when I start toying with .43.

@bep
Copy link
Member Author

bep commented Jun 28, 2018

I have reverted my thoughts on the template part of this. I added resources.FromString more as a way of testing this without having to create all these files, but I think it can be useful for the common man, too. But resources.ExecuteAsTemplate is a "resource transformer`, e.g.

{{ $r :=  resources.Get "my.template" | resources.ExecuteAsTemplate .Site }}

@regisphilibert
Copy link
Member

I see just like partial, its one argument is context.

@clipperhouse
Copy link

Going back to the Concat vs ConcatTo thread above, perhaps Concat can have a sensible default for the file name, so the user doesn’t even need to pass it?

{{ resources "*.scss" | toCSS | concat | fingerprint | permalink }}

Optimizing for terseness and clarity for the simplest use cases.

The permalink might be something like concat.{hash}.css, but it doesn’t matter much, since we just need to link to it.

@bep
Copy link
Member Author

bep commented Jun 28, 2018

@clipperhouse the only sensible default would be UUID or something unique for an extension. But that sounds like adding a pile of work to ... a pile. For not a huge amount of value. I personally would really want to control the URL of my concatenated resources.

@bep
Copy link
Member Author

bep commented Jul 3, 2018

Hey @regisphilibert @kaushalmodi @lucperkins @onedrawingperday and gang:

I have updated the table in my first post in this thread to match the current -- and possibly final -- name scheme. All the functions marked with a green check is implemented. The others can wait. Any final complains or suggestions? (and please don't make it a "could you also add ...")

See #4854 (comment)

I have some support work to do in this area (build scripts etc.) that needs to be done, but other than that the implementation is more or less solid.

@kaushalmodi
Copy link
Contributor

Any final complains or suggestions?

Looks awesome!

@regisphilibert
Copy link
Member

So everything is ready but two functions :) Awesome! Can't wait to try it out!

resources.Get is all we need for the vas majority of use cases, I'm still very excited for the other ones though, 'cause uses cases generally comes up once the feature is there :)

@bep
Copy link
Member Author

bep commented Jul 3, 2018

resources.Get is all we

I'm not sure who you include in the "we" above, but I suspect this will be very individual. To me, the biggest part of this is the SCSS -- and if you combine that with resources.ExecuteAsTemplate and themes ... WHoa.

@regisphilibert
Copy link
Member

I'm not sure who you include in the "we" above

I guess I mean the early adopters. More to the point, I meant we can live without .GetMatch and .Match for now. I agree SCSS is the biggest seller here. Personally I can't wait to set up an node free Hugo project.

@rdwatters
Copy link
Contributor

Curious as to whether you have decided to implement the aliases you mentioned above as well for this particular set of transformers, @bep.

@bep
Copy link
Member Author

bep commented Jul 5, 2018

Curious as to whether you have decided to implement the aliases you mentioned above as well for this particular set of transformers, @bep.

I have.

@regisphilibert
Copy link
Member

This is about naming so I'll write about this here instead of #4858. The postCSS method seems to deal only with autoprefixer, but postCSS unless I am wrong is a tool to build your css using plugins lots of... in a Gulp or Grunt environment. So it seems to do much more than autoprefixing.

Unless I misunderstood the purpose of resources.PostCSS , is it the right name? Can't we just call it resources.Autoprefix or something?

@jbrodriguez
Copy link
Contributor

Hi, this is an awesome feature to have.

A native asset pipeline processor is great !

I hope some docs are included to get a better picture on how to invoke it on a whole site and such.

Also, I don't see references to image optimization, although I don't seem to find a go implementation of this kind of feature either.

In any case, great job !

@regisphilibert
Copy link
Member

regisphilibert commented Jul 5, 2018

Also, I don't see references to image optimization

It will work with Hugo's current Image Processing

@bep
Copy link
Member Author

bep commented Jul 5, 2018

Can't we just call it resources.Autoprefix

That's limiting as PostCSS has a myriads of plugin you can use. We try to find the postcss config file either in the project or theme, or you give a path when you do the transformation. This will be clear once I write some docs ...

@bep
Copy link
Member Author

bep commented Jul 5, 2018

@regisphilibert
Copy link
Member

We try to find the postcss config file either in the project or theme, or you give a path when you do the transformation.

Damn. This is is huge! I did misunderstand .PostCSS. Thanks for clarifying.

@onedrawingperday
Copy link
Contributor

Looks awesome @bep

I’ve been away on a holiday but I’ll be back just in time for the release.

@stale
Copy link

stale bot commented Nov 3, 2018

This issue has been automatically marked as stale because it has not had recent activity. The resources of the Hugo team are limited, and so we are asking for your help.
If this is a bug and you can still reproduce this error on the master branch, please reply with all of the information you have about it in order to keep the issue open.
If this is a feature request, and you feel that it is still relevant and valuable, please tell us why.
This issue will automatically be closed in the near future if no further activity occurs. Thank you for all your contributions.

@stale stale bot added the Stale label Nov 3, 2018
@bep bep closed this as completed Nov 3, 2018
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests