search

CircleCI-Archived / stefon

An asset pipeline for clojure
98 stars 16 forks source link

Add uri-root option #12

Closed RyanMcG closed 10 years ago

RyanMcG commented 11 years ago

This option allows the generated uris in production to have a prefix to their asset root. The purpose is to generate uris in production that are not served at the site root.

I wanted this feature for generating assets for a gh-pages site. Since those are served from a subdirectory, The uris generated by link-to-asset were not working since they start at site root (i.e./).

Fixes #10.

pbiggar commented 11 years ago

When I read your issue, I thought it was going to replace "assets" with a different root.

RyanMcG commented 11 years ago

Hmm, while that could be valuable it would require a more involved solution than I needed.

pbiggar commented 11 years ago

I'm not sure this feature is really valuable without it, TBH.

RyanMcG commented 10 years ago

Replacing assets with different root could be useful. Let's examine what that would mean:

The feature: Allow specifying an arbitrary path to be the root of stefon generated files Why?: Only allowing /assets/ could cause conflicts if a user is trying to include

Upon further consideration while this feature does not solve my use-case directly if it were in place I could hack around a bit to get it working with my client project. This seems fair since my client project is not a typical use-case.

I'll update this PR.

RyanMcG commented 10 years ago

Hmm, hold off on this for now. Though it supports the use case you imagined it does not work for mine. Can you tell me why we save and load a manifest file?

EDIT: It seems to make this work I'll have to modify what manifest is storing to be just the md5 hash. Really, that's all it needs to store since the rest can be quickly determined programmatically.

RyanMcG commented 10 years ago

I think the original fix is actually better. Modifying the generated assets root can be another feature though. It's an easy one. However, due to the way we cache the entire uri in the manifest and not just the digest or the adrf with a with a digest I can not manipulate the feature for my use case without changing what we store in the manifest.

Another reason the uri-root feature might be a good idea is that other use cases exist for it. Consider a ring app behind a proxy. The proxy routes requests to a directory in the domain to the ring app. This could be serving these static assets or it could not be (leaving the ring app to do it).

Two possibilities:

  1. A request to this domain would get routed by the proxy to the ring app if the route is prefixed by /app/
  2. A request to this domain would get routed by the proxy to the ring app if a static file does not exist in the some server root and it is prefixed by /app/

The original uri-root option I proposed supported use case 1. Simply allowing the generated-assets-root to be modified supports use case 2. The problem is that uri's generated by stefon the app is rooted at domain root. By changing generated-assets-root to "/app/assets/" we would generate the proper uris with link-to-asset and the files would be written to the directory structure /app/assets in the serving root. With case 2 this is correct. With case 1 putting them in just /assets/ is probably correct (it depends on how the proxy delivers requests to ring app (does it remove the prefix from the route or not?).

Anyway, I am going to move these changes to another PR with an appropriately named branch and revert this PR back to its original state.

RyanMcG commented 10 years ago

Any new thoughts?

RyanMcG commented 10 years ago

I've decided to withdraw this PR.

I found that there are rather easy workarounds which do not require modifying stefon. Specifically, using Hiccup's with-base-url macro seems like a better way to do this anyway.

pbiggar commented 10 years ago

@RyanMcG cool, thanks. that's a good resolution I think: we dont want to extend stefon for everything I guess.