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

Issues having a file that matches manifest-*.json in your assets directory #229

Closed
juddblair opened this issue Mar 17, 2015 · 7 comments
Closed

Comments

@juddblair
Copy link

So, we had a JSON manifest we were using for our favicons following this W3C draft - https://w3c.github.io/manifest/ . The file was named manifest.json, and was living alongside the icon assets in assets/images.

What ends up happening is this file gets precompiled by the asset pipeline, and you now have:
manifest-<some hash>.json
manifest-<some other hash>.json
in your resulting public/assets directory

As best I can tell, when you start the app, whatever .first returns from all files that match manifest-*.json is used as the asset pipeline manifest, which ends up causing asset helpers to return paths to uncompiled assets when the wrong manifest is selected. The matcher is extra confusing because even if you renamed that file, say manifest-favicons.json, it would still match and exhibit the same behavior.

Seems like precompilation should fail when this situation is detected.

(and yeah, we're doing it wrong since static assets should live in /public, but this still seems like a pretty dangerous gotcha)

@josh
Copy link
Contributor

josh commented Mar 17, 2015

Yeah, I'd really like to rename that file.

First it should probably be a .something.json. I think that will also fit into many default apache/nginx configs to prevent it from being served. Then maybe naming it something like .sprockets-mainfest.json would go even further.

wdyt?

@rafaelfranca
Copy link
Member

👍 for .sprockets-manifest.json

@josh
Copy link
Contributor

josh commented Mar 17, 2015

The trick is probably going to be transitioning away from the old name. There definitely has to be a good upgrade path here.

This 3.x beta seems like a good time. We can probably support both styles on 3.x and only this new one on 4.x.

@juddblair
Copy link
Author

Yup, I think that's a good approach, and won't clash with any assets you actually want served. 👍 here as well.

@josh
Copy link
Contributor

josh commented Mar 17, 2015

Cool.

I'll probably take a look at this sometime over this weekend. Otherwise, if someone wants to get a PR started sooner, I'm happy to help up and give direction.

@josh
Copy link
Contributor

josh commented Mar 17, 2015

Ah, I just realized this is on the sprockets-rails issue tracker. The change should be made to sprockets core. I'll open a cross referencing issue over there.

@rafaelfranca
Copy link
Member

This is fixed at sprockets 3.

leppert added a commit to reading-am/reading that referenced this issue May 31, 2015
leppert added a commit to reading-am/reading that referenced this issue Jun 6, 2022
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

No branches or pull requests

3 participants