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

Static Asset Management #1405

Closed
ErisDS opened this issue Nov 5, 2013 · 23 comments

Comments

@ErisDS
Copy link
Member

commented Nov 5, 2013

It's time to upgrade Ghost's static asset system to something a little more heavy duty, so that we can handle more use cases, and also add proper cache control headers as discussed at length in issue #932 (closed in favour of this issue).

We need to be able to serve static assets for

  • Ghost admin
  • themes
  • apps
  • images

This should entirely replace or wrap our current usage of connect static.

We still need to be able to hot-swap themes.

This needs to be abstracted so that it uses local file storage by default, but so that we can provide a way for apps to override this. They may wish to use a CDN, or store files elsewhere.

Static assets need to be versioned so that the cache can be invalidated. This should be done using a hash of the file which is appended to the filename, something like (ghost.js -> ghost.aba23928.js)

We'll need a helper for outputting script and style tags for managed assets both in themes, and in the Ghost admin, something like:

  • {{asset "css/screen.css"}} for themes, and if you're on Ghost it will look like
  • {{asset "css/screen.css" ghost="true"}} (or it could be {{asset "css/screen.css" "ghost"}})

It would be nice if this looked for a specific (assets/css, assets/js) structure by default, encouraging theme developers to structure their theme in a consistent way, but could be overridden.

@vvo

This comment has been minimized.

Copy link

commented Nov 11, 2013

Hello.

One quick note, I have been fighting with my digitalocean ghost droplet to find out why it was no caching even CSS/JS files.

What I found out is that the current release of ghost sends a Cache-Control: public, max-age=0 header on assets.

This was a big "OH MY!!", static assets should always be cached, at least for some time.

Don't you think we should first set some cache on static assets. Setting max-age=0 on static assets is a reaaaally bad idea.

I know that you are working on handling static assets the "right way".

But in the meantime, we could set some cache on static assets even if we do not have an easy way to wipe out the cache (eg, query string, md5 filenames).

Thank you

@ErisDS

This comment has been minimized.

Copy link
Member Author

commented Nov 11, 2013

@vvo you make a fair point, but there is no 'in the meantime' - the next version of Ghost will be 0.4 which will have proper asset management.

ErisDS referenced this issue in ErisDS/Ghost Nov 11, 2013
First pass at correctly configured Cache-Control headers
issue TryGhost#932

- sets cache control headers on all routes
- is a bit aggressive in places
@WraithKenny

This comment has been minimized.

Copy link

commented Nov 14, 2013

H5BP has something that might spark some ideas https://github.com/h5bp/server-configs-node and it's an express middleware.

@ErisDS

This comment has been minimized.

Copy link
Member Author

commented Nov 14, 2013

@WraithKenny Neat! Thanks 👍

@ghost ghost assigned bastilian Nov 26, 2013

@ErisDS

This comment has been minimized.

Copy link
Member Author

commented Nov 28, 2013

Other related reading: #670

@bastilian

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2013

I read through a couple of issues/discussions, @ErisDS mentioned and want to summarise everything before getting started.

I also looked at three potential solutions:

I am leaning more towards asset-rack. All solutions offer the same in functionality, however connect-assets and connect-mincer have conventions to follow, these could be wrapped and bent to the needs of Ghost, because Ghost already has certain conventions where files are. Both lack the ability to set headers, for specific assets, again these could be wrapped, but i'd rather use a solution that already offers that.
Also, asset-rack has a very clean way of defining assets.

The only thing that asset-rack is missing is a way of "precompiling" assets. By default they will be kept in memory and served from there, actually a good thing. When a CDN is set it automatically deploys them there, when Ghost is started. Therefore we need to add the ability to precompile (copy them to the local filesystem) them with the same conventions as asset-rack does.

Assets to be served:

  • Theme assets (/content/theme/THEME_NAME)
    • Blacklisting file-types should be possible, like .txt, .hbs, .md, not to be served.
  • Uploaded assets/images (/content/images)
    • not hashed
  • Ghost-Admin scripts (/core/built/scripts)
  • Ghost-Admin assets (/core/client/assets)
  • Shared assets:
    • /core/shared/img
    • /core/shared/favicon.ico (not hashed)

Note: when saying root-path in the following, it is considered a given, that if ghost lives in a subdir, that this is the root-path. :)

  • Theme assets and Uploaded images are served under the root
  • Uploaded images will not be hashed, since they live in the content and we currently are not able to automatically adjust their url
  • "Ghost-Admin" assets will be served under /ghost (the file location in Ghost, stays the same)
  • Appropriate Cache-Headers will be set as mentioned in #1470

Hashing asset filenames

All three above mentioned solutions offer appending a md5 hash to the filename out of the box.

All use the file-contents to generate, this means, that even if for example a theme developer, for whatever reason, happens to create a file with the path "/ghost/css/screen.css", nothing will be influenced, since chances are very slim, that this file will have the same md5 as Ghost's screen.css.

Appending the md5-hash makes it also possible to use the same asset-server or CDN for multiple Ghost installations. As mentioned above, even if assets happen to have the same path and filename, since we append a hash generate from it's content, it needs to be exactly the same file to have the same md5 hash.

Hashing assets will only happen in the production environment.

Building assets

Compiling and/or minifying assets is not part of this issue. Grunt will take care of this. All three packages have compiling and minifying built in, but it will be kept separate as discussed in #670.

Am I missing something? (I could, I have the feeling, I do...)

@bastilian

This comment has been minimized.

Copy link
Contributor

commented Nov 28, 2013

Oh, the asset-helper. :)

@halfdan already implemented (

// ### Asset helper
) will be adjusted to use the helpers provided by the package, which takes hostname and path into consideration.

@ErisDS

This comment has been minimized.

Copy link
Member Author

commented Dec 1, 2013

I don't think you've missed anything :)

A couple of notes:

Uploaded images will not be hashed, since they live in the content and we currently are not able to automatically adjust their url

This is ok for now as we enforce a unique filename and no overwrites :)

the asset helper

Must not assume anything about where within a theme the assets live. Some themes use an assets folder, some use css/js folders directly. I would like to assume an assets folder, and fall back if it doesn't exist, but that may be overly complex.

@bastilian

This comment has been minimized.

Copy link
Contributor

commented Dec 1, 2013

I already have a working asset-rack branch serving assets locally. it has a few issues, mainly with theme switching, when i have that fixed, i'll push it. probably tomorrow.

Uploaded Images: actually, it would be possible to return the url generated by asset-rack when uploaded and already uploaded images would be served under the existing name.

Asset helper: It is no big thing actually in the helper to assume a asset folder. I check if an asset exists in the asset-collection, if not i try with the non-"/asset/" prefixed url.

@halfdan

This comment has been minimized.

Copy link
Member

commented Dec 1, 2013

@bastilian, the check for asset/ should be cached so the file system won't be hit that often.

@bastilian

This comment has been minimized.

Copy link
Contributor

commented Dec 1, 2013

@halfdan the assets are cached. :)

@ErisDS

This comment has been minimized.

Copy link
Member Author

commented Dec 30, 2013

As the asset-rack solution isn't finished, and seems to introduce some significant slow-down, I've implemented query-string asset management for the time being, and we'll look at this again in 0.5

ErisDS added a commit to ErisDS/Ghost that referenced this issue Jan 2, 2014
Cache control headers & query string asset management
closes TryGhost#1470
issue TryGhost#1405

- added cache control middleware
- added defaults for all routes, assets, etc
- updated asset helper to add a query string with a timestamp hash to all assets
- added unit tests for asset and ghostScriptTags helpers
- added cache-control checks to route tests

@ErisDS ErisDS added server and removed maybe0.5 labels Feb 26, 2014

@sebgie

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2014

As we are hovering around this issue for quite some time and broccoli has an issue with windows I wanted to throw another library into the ring: https://github.com/mediocre/electricity. Could someone who has more experience with asset management than me please take a look if that could eventually serve our needs?

@chirag04

This comment has been minimized.

Copy link

commented Apr 21, 2014

I am not totally sure of the problem here but asset-smasher can be worth having a look.

https://github.com/jriecken/asset-smasher

cc @ErisDS @sebgie

@karai17

This comment has been minimized.

Copy link

commented Nov 5, 2014

I've been using Ghost for a couple weeks now and I've already come to find a lack of asset management a bit frustrating. I see this issue is already a year old, which is a little discouraging, but I must insist that asset management is a very important feature.

Case 1: A user uploads a photo then decides to use another. The only current way to remove the old asset is to dig around the server via FTP. This is far from optimal.

Case 2: A user uploads a photo, realizes it is the wrong one, and wants to replace it. This is not possible without again digging around the filesystem via FTP.

Case 3: A user wants to remove an image but is unaware that it is being used elsewhere on the blog, perhaps by another user. There is no way for a user to determine if a file is being actively linked to, nor is there a warning to the user who originally made a post using that file that their post is now broken.

Asset management is a huge UX boost and allows teams of bloggers to work together without destroying each other's work, much like version control.

@nuclearpengy

This comment has been minimized.

Copy link

commented Nov 5, 2014

Hi @karai17 and welcome to Ghost. :-)

If you're interested in getting insight into the development cycle to see what's coming next, take a look at the product roadmap on Trello: https://trello.com/b/EceUgtCL/ghost-roadmap

Advanced Media Management is still on the backlog.

@karai17

This comment has been minimized.

Copy link

commented Nov 5, 2014

I'm aware that it is on the backlog, but I really think it is a feature worth more consideration then "when we get around to it". A user should not have to spend any time in the filesystem when dealing with something as mundane as managing assets.

@oluseyi

This comment has been minimized.

Copy link

commented Nov 5, 2014

This is the nature of open source. Everyone has a different top priority, but unless they are prepared to actually do the work or pay someone to, they just have to accept the rate at which work gets done by others.

'Twas ever thus, and ever thus 'twill be.

On Nov 5, 2014, at 5:42 PM, Landon Manning <notifications@github.commailto:notifications@github.com> wrote:

I'm aware that it is on the backlog, but I really think it is a feature worth more consideration then "when we get around to it". A user should not have to spend any time in the filesystem when dealing with something as mundane as managing assets.

Reply to this email directly or view it on GitHubhttps://github.com//issues/1405#issuecomment-61895229.

@karai17

This comment has been minimized.

Copy link

commented Nov 5, 2014

Yeah, I'm just throwing my voice in. I haven't done webdev work in long enough that I'd even begin to feel comfortable contributing code to Ghost, so the best I can do is make my suggestions.

@ErisDS

This comment has been minimized.

Copy link
Member Author

commented May 19, 2015

Closing this. At some point we'll probably want to add a much more extensive asset pipeline to Ghost, but what we have pretty much works for now. When it's time to add something better, we'll get it spec'd out in more detail.

@ErisDS ErisDS closed this May 19, 2015

@ErisDS ErisDS removed this from the Future Backlog milestone May 19, 2015

@smasher164

This comment has been minimized.

Copy link

commented Sep 28, 2015

I'm having trouble with browser caching of my assets. Because browsers don't cache files with a query string ?v=, I think a possible fix would be to rev the file names differently. Instead of mylogo.gif?v=1.2, we should use mylogo.1.2.gif. This provides both the advantage of cache busting, as well as proxy caching of assets.

Looking through the helpers/ directory, Ghost/core/server/helpers/utils.js seems to be inserting the query string <%= source %>?v=<%= version %>. However, I'm not sure how to change it because <%= source %> contains the file extension.

@ErisDS

This comment has been minimized.

Copy link
Member Author

commented Oct 8, 2015

Because browsers don't cache files with a query string

To the best of my knowledge, all modern browsers do cache files with a query string. Some network-level caches do not. Modifying the filename instead of adding a query string is a non-trivial change that would require a significant reworking of file lookup.

@vvo

This comment has been minimized.

Copy link

commented Oct 8, 2015

💯 to what @ErisDS said, browser do cache query strings. Still, not using query string is safer when behing proxies/CDNs etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.