Skip to content

Commit d6156db

Browse files
committed
Trim cache length in cachePop, not just cachePush
This fixes a bug where the `pjax.defaults.maxCacheLength` setting was being applied when navigating to new pages but not when users hit the back or forward buttons. Two symptoms of this were: * Setting `maxCacheLength = 0` didn't completely disable caching; hitting the back button followed by the forward button would ask the server for the first page but read the second one from the cache. I think this is probably the bug that was discussed on issue #406. * Regardless of what you set `maxCacheLength` to, the forward cache would continue growing as long as you kept hitting the back button.
1 parent cc0dd17 commit d6156db

File tree

2 files changed

+137
-7
lines changed

2 files changed

+137
-7
lines changed

jquery.pjax.js

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -744,14 +744,11 @@ function cachePush(id, value) {
744744
cacheMapping[id] = value
745745
cacheBackStack.push(id)
746746

747-
// Remove all entires in forward history stack after pushing
748-
// a new page.
749-
while (cacheForwardStack.length)
750-
delete cacheMapping[cacheForwardStack.shift()]
747+
// Remove all entries in forward history stack after pushing a new page.
748+
trimCacheStack(cacheForwardStack, 0)
751749

752750
// Trim back history stack to max cache length.
753-
while (cacheBackStack.length > pjax.defaults.maxCacheLength)
754-
delete cacheMapping[cacheBackStack.shift()]
751+
trimCacheStack(cacheBackStack, pjax.defaults.maxCacheLength)
755752
}
756753

757754
// Shifts cache from directional history cache. Should be
@@ -778,6 +775,21 @@ function cachePop(direction, id, value) {
778775
pushStack.push(id)
779776
if (id = popStack.pop())
780777
delete cacheMapping[id]
778+
779+
// Trim whichever stack we just pushed to to max cache length.
780+
trimCacheStack(pushStack, pjax.defaults.maxCacheLength)
781+
}
782+
783+
// Trim a cache stack (either cacheBackStack or cacheForwardStack) to be no
784+
// longer than the specified length, deleting cached DOM elements as necessary.
785+
//
786+
// stack - Array of state IDs
787+
// length - Maximum length to trim to
788+
//
789+
// Returns nothing.
790+
function trimCacheStack(stack, length) {
791+
while (stack.length > length)
792+
delete cacheMapping[stack.shift()]
781793
}
782794

783795
// Public: Find version identifier for the initial page load.

test/unit/pjax.js

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,7 @@ if ($.support.pjax) {
891891
// Test is fragile
892892
asyncTest("no initial pjax:popstate event", function() {
893893
var frame = this.frame
894-
var count = 0;
894+
var count = 0
895895

896896
window.iframeLoad = function() {
897897
count++
@@ -941,6 +941,124 @@ if ($.support.pjax) {
941941
window.iframeLoad()
942942
})
943943

944+
asyncTest("navigation through history obeys maxCacheLength", function() {
945+
var frame = this.frame
946+
var count = 0
947+
948+
// Reduce the maxCacheLength for this spec to make it easier to test.
949+
frame.$.pjax.defaults.maxCacheLength = 2
950+
951+
// We can determine whether a navigation event was fulfilled by the cache
952+
// or the server by checking whether pjax:beforeSend fired before pjax:end.
953+
var navigationWasCached
954+
955+
frame.$("#main").on("pjax:beforeSend", function() {
956+
navigationWasCached = false
957+
})
958+
959+
frame.$("#main").on("pjax:end", function() {
960+
count++
961+
962+
// First, navigate three times.
963+
if (count == 1) {
964+
equal(frame.location.pathname, "/hello.html")
965+
navigationWasCached = true
966+
frame.$.pjax({url: "env.html", container: "#main"})
967+
} else if (count == 2) {
968+
equal(frame.location.pathname, "/env.html")
969+
equal(navigationWasCached, false)
970+
navigationWasCached = true
971+
frame.$.pjax({url: "hello.html", container: "#main"})
972+
} else if (count == 3) {
973+
equal(frame.location.pathname, "/hello.html")
974+
equal(navigationWasCached, false)
975+
navigationWasCached = true
976+
frame.$.pjax({url: "env.html", container: "#main"})
977+
} else if (count == 4) {
978+
equal(frame.location.pathname, "/env.html")
979+
equal(navigationWasCached, false)
980+
// The cache should now have two items in it, so hitting the back
981+
// button three times should pull the first two pages from cache and
982+
// the third from the server.
983+
navigationWasCached = true
984+
frame.history.back()
985+
} else if (count == 5) {
986+
equal(frame.location.pathname, "/hello.html")
987+
equal(navigationWasCached, true)
988+
frame.history.back()
989+
} else if (count == 6) {
990+
equal(frame.location.pathname, "/env.html")
991+
equal(navigationWasCached, true)
992+
frame.history.back()
993+
} else if (count == 7) {
994+
equal(frame.location.pathname, "/hello.html")
995+
equal(navigationWasCached, false)
996+
// The cache should still have two items in it, both in the forward
997+
// cache. If we hit forward three times, the first two should come from
998+
// cache and the third from the server.
999+
navigationWasCached = true
1000+
frame.history.forward()
1001+
} else if (count == 8) {
1002+
equal(frame.location.pathname, "/env.html")
1003+
equal(navigationWasCached, true)
1004+
frame.history.forward()
1005+
} else if (count == 9) {
1006+
equal(frame.location.pathname, "/hello.html")
1007+
equal(navigationWasCached, true)
1008+
frame.history.forward()
1009+
} else if (count == 10) {
1010+
equal(frame.location.pathname, "/env.html")
1011+
equal(navigationWasCached, false)
1012+
start()
1013+
}
1014+
})
1015+
1016+
equal(frame.location.pathname, "/home.html")
1017+
frame.$.pjax({url: "hello.html", container: "#main"})
1018+
})
1019+
1020+
asyncTest("setting maxCacheLength to 0 disables caching", function() {
1021+
var frame = this.frame
1022+
var count = 0
1023+
1024+
frame.$.pjax.defaults.maxCacheLength = 0
1025+
1026+
// We can determine whether a navigation event was fulfilled by the cache
1027+
// or the server by checking whether pjax:beforeSend fired before pjax:end.
1028+
var navigationWasCached
1029+
1030+
frame.$("#main").on("pjax:beforeSend", function() {
1031+
navigationWasCached = false
1032+
})
1033+
1034+
frame.$("#main").on("pjax:end", function() {
1035+
count++
1036+
1037+
if (count == 1) {
1038+
equal(frame.location.pathname, "/hello.html")
1039+
navigationWasCached = true
1040+
frame.$.pjax({url: "env.html", container: "#main"})
1041+
} else if (count == 2) {
1042+
equal(frame.location.pathname, "/env.html")
1043+
equal(navigationWasCached, false)
1044+
navigationWasCached = true
1045+
frame.history.back()
1046+
} else if (count == 3) {
1047+
equal(frame.location.pathname, "/hello.html")
1048+
equal(navigationWasCached, false)
1049+
navigationWasCached = true
1050+
frame.history.forward()
1051+
} else if (count == 4) {
1052+
equal(frame.location.pathname, "/env.html")
1053+
equal(navigationWasCached, false)
1054+
start()
1055+
}
1056+
})
1057+
1058+
equal(frame.location.pathname, "/home.html")
1059+
frame.$.pjax({url: "hello.html", container: "#main"})
1060+
})
1061+
9441062
asyncTest("popstate preserves GET data", function() {
9451063
var frame = this.frame
9461064

0 commit comments

Comments
 (0)