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

Fix JavaScript caching and put together sys admin recommendations #192

Closed
jerboaa opened this issue Nov 21, 2010 · 13 comments
Closed

Fix JavaScript caching and put together sys admin recommendations #192

jerboaa opened this issue Nov 21, 2010 · 13 comments
Labels
Milestone

Comments

@jerboaa
Copy link
Member

jerboaa commented Nov 21, 2010

Have a look at this:
http://apidock.com/rails/ActionView/Helpers/AssetTagHelper/javascript_include_tag

plus comments.IIRC we are using :cache => true which makes it cache to a file called all.js. We could advise sys admins to create that file and make it writable by mongrels. This might convince them to turn JS caching on.

More on this in old #176.

@benjaminvialle
Copy link
Member

Yes, I am having this issue too with Passenger. We may do something wrong in the way we cache things… I don't know what is happening here O_o

@benjaminvialle
Copy link
Member

Some screenshots of what is happening in production mode with Passenger (and I guess it is the same thing in Mongrel):

http://bvialle.eu/pub/file.php?h=Raf2b96f086f06b363ed099b43f6ef5f0

http://bvialle.eu/pub/file.php?h=Ra710cd8569ae1c4dbc00c2f18d34b51f

@jerboaa
Copy link
Member Author

jerboaa commented Apr 27, 2011

I can't view your screenshots, unfortunately. It gives me the php file, which - when downloaded - I can't open. I think the cleanest solution to this is to create a directory in public/javascripts, say "cache" and let Rails write there if the system admin is comfortable with that.

In a proxied setup this breaks things too. At least the way we recommend the setup to be. We need to put more thought into this JS caching issue :)

@benjaminvialle
Copy link
Member

Btw, it is a bug in my file uploader system. But try to open it with an image editor, like eog for example. It works :)

@jerboaa
Copy link
Member Author

jerboaa commented Apr 29, 2011

Nice bug ;-) I guess you're not setting the mime-type properly. And yes, that looks very much like something I've seen before. I confirm, it's applicable to a mongrel setup too.

@jerboaa
Copy link
Member Author

jerboaa commented May 12, 2011

Benjamin, I had a brief look at this and it appears that all the Rails :cache thing is doing for javascript/stylesheet tags is concatenate them together into one file. It does not strip comments or the like. I'm not really seeing what the big advantage of this is. At the moment the best way I can think of handling the read-only situation is to run MarkUs with js caching turned on, let rails produce those files and then ship them (the cached files) in our release. Overall it seems to be a bit backwards, though. Does anybody have a better idea?

BTW: I was able to reproduce the scroll-bar issue in the grader view. Key is to turn JS caching on. Not sure why. I fixed a few styles and it looks ok now. I'll post a review soon.

@benjaminvialle
Copy link
Member

Ok. Severin, what we could do is, for the moment, deactivate javascript caching by defaut (I think it is enabled for production by default) and move this issue once we will have move to Rails3. There must be better mechanisms with Rails3 and it may simplifies things.

I know we can keep all ":cache => " lines in the code if we only deactivate javascript in config/environments/*.rb environments.

What do you think ?

@jerboaa
Copy link
Member Author

jerboaa commented May 15, 2011

That would be an option. I would rather get rid of our :cache directives.

$ grep -rn ':cache' app | wc -l
13

There aren't too many anyways. Then we should revisit caching in a more serious approach. I'm reluctant to ship with config.action_controller.perform_caching = false by default, since this turns off other caching as well. Caching best practices is worth an entire wiki page on its own (at the very least) and we shouldn't take this lightly. For a 1.0 we could come with config examples for memcached, ship with minified/restructured javascript files (they are scattered all over the place in public/javascripts), etc. There is lots we can do. Do you think that would be OK?

@benjaminvialle
Copy link
Member

Ok. So go for it. :) We only remove javascript caching for the 0.10.0 release.

@jerboaa
Copy link
Member Author

jerboaa commented May 15, 2011

Ok. I'll keep this issue open and set the milestone to 1.0, though.

@reidka
Copy link
Member

reidka commented May 20, 2014

Is it time to review js caching?

@reidka reidka modified the milestones: 1.2 , 1.1 May 20, 2014
@benjaminvialle
Copy link
Member

Maybe better after JS cleanup (see #1496)

@david-yz-liu
Copy link
Contributor

I'm not really sure that this issue is still relevant; @reidka I'm going to close it for now.

But perhaps I don't understand the problem; a clearer description might be helpful too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants