Closed (fixed)
Project:
JavaScript Libraries Manager
Version:
7.x-1.0-rc2
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
29 Nov 2011 at 00:49 UTC
Updated:
30 Dec 2011 at 15:30 UTC
Jump to comment: Most recent file
Comments
Comment #1
msonnabaum commentedAttached patch adds an option to cache locally if the script is external.
The http request is only fired if the file isn't already cached. I stole the function from google analytics which works this way.
I also added a cron hook to ensure that the cache is always primed.
Comment #2
David_Rothstein commentedSeems like the function should return file_create_url() somewhere in the "if" statement also?
Comment #3
pwolanin commentedI think it depends. Our assumption is that users in general visit only 1 site, so reducing http requests may be much more important than using a shared resource.
If the resource is e.g. a twitter or other JS file used commonly on the web it might make sense not to do this on the assumption that some fair fraction of browsers will have it cached already.
Comment #4
pwolanin commentedFeels like this is missing something.
If we have a cached copy it should be included in aggregated JS. however, if the JS is not aggregated, we ideally would use the external URL?
Comment #5
pwolanin commentedRevised patch. not really tested yet, but I think the overall logic is improved.
Comment #6
pwolanin commentedhow much overhead is the extra N file stats per page load?
Comment #7
pwolanin commentedRevised patched, reduces code duplication, improves logic. Quick local testing looks good.
Comment #8
pwolanin commentedA couple more improvements to the admin form
Comment #9
pwolanin commentedimprove hook_cron. No need to run the command expect once per day to sync.
Comitting this patch. Open new issues for follow-up.