adobe / brackets.io

brackets.io website
111 stars 80 forks source link

Prevent update notifications from being cached #97

Closed ingorichter closed 10 years ago

ingorichter commented 10 years ago

Added timestamp parameter to ajax request to prevent caching. There are several ways of getting this feature, but this was the one with the least changes and it doesn't really differ from what the generic $.ajax call is doing by specifying {cache: false} in the request options.

ingorichter commented 10 years ago

@peterflynn I would appreciate if you could look at this little change. Thanks.

marcelgerber commented 10 years ago

@ingorichter Well, changing it to an $.ajax call is not much change either, and it would be more self-explanatory when looking at the code.

peterflynn commented 10 years ago

It is weird -- if I hit the page with dev tools opened, it still pings the S3 URL, but S3 returns 304 which tells Chrome to use its cached copy. You'd think pushing a new JSON file would cause it to stop returning 304, since the file's timestamp & etag has changed. So maybe there's an underlying issue with our S3 config too.

This seems reasonable as a workaround, though.

peterflynn commented 10 years ago

@ingorichter Is it worth adding a comment just above this line explaining that we add this param to ensure we never get a cached copy of the JSON?

TomMalbran commented 10 years ago

I agree with @MarcelGerber. We are already using $.ajax for the extensions registry, so it will fit nicely here. Or we could have our own utils function that returns the result from $.ajax({url: url, dataType: "json", cache: false})

ingorichter commented 10 years ago

Let me change this to $.ajax.

ingorichter commented 10 years ago

@peterflynn Updating the json should change the e-tag and the timestamp. I don't know what else could be going on here, but at least the timestamp on the json file is reflecting the date from last Friday. I can't tell for the etag, since this doesn't let you conclude any information from it.

ingorichter commented 10 years ago

@peterflynn BTW: in my case the release notes at the bottom middle column got never updated. Now with my change, it is updating properly. I haven't thought about it, that this could be an caching issue...

ingorichter commented 10 years ago

Okay, I changed the call from $.getJson to $.ajax

ingorichter commented 10 years ago

Nope, that wasn't intended. Thanks.

peterflynn commented 10 years ago

Looks close enough to me! Merging...