Skip to content

Commit 3abd2b1

Browse files
hedgepigdanielhedgepigdaniel
authored
fix(history): store index of current history stack entry in the browser location state (#61)
Previously the history stack entries contained only an `id` referring to a history, while the entries and current index were stored in session storage. As in #59, this is not enough information to reliably determine which entry is currently active. For example, when the user uses the browser back/forward buttons (which can travel across multiple history states at once). Here the current index has been moved from the session storage to the state of each browser history entry. Since the combination of session ID and session entry index uniquely identifies and entry, the correct entry can be selected from the saved stack in session storage in all circumstances, given the contents of the current history state which can be read from the browser. ## Specific changes - Remove behaviours for browsers not supporting the history API or session storage. They didn't work in the same way, and those browsers are now very very old (e.g. IE9) - Remove workaround for an edge case where navigating away from the app and back again could cause Rudy to autonomously navigate away from the app a second time. The workaround didn't work in all cases (e.g. if the user manually typed a new URL into the address bar), and is no longer necessary now that the underlying bugs is fixed. - Remove the behaviour of clearing subsequent entries in the saved history stack when loading the app fresh somewhere in the middle of the stack. The logic underlying this is not sound, because it is possible to traverse multiple entries in the history stack (arriving at entry 2/5 does not mean that entries 3-4 are not still in the future). This way it is still possible for there to be future entries in the rudy/redux state that don't correspond with the browsers future states, but a least navigation will always work correctly. Trying to go forwards via rudy beyond the history entries that the browser agrees with still works, in the sense that it correctly navigates to wherever is next in the browser's history stack (Rudy has no choice about that) - Replace existing logic to guess the direction/distance of a browser initiated pop now that the correct index is available in the pop state - Add a new test covering browser initiated pops, including where multiple entries are traversed at once BREAKING CHANGE: browsers that don't support the history API or session storage are no longer supported at all. In practice this means IE9 is no longer supported. BREAKING CHANGE: future entries in the history stack remain even if the user has since navigated from a previous entry to an external site (because they are no longer removed when they are potentially still the future entries in the browser stack) Fixes #59
1 parent 51ff994 commit 3abd2b1

46 files changed

Lines changed: 1891 additions & 3005 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎packages/integration-tests/__helpers__/awaitUrlChange.js‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,25 @@ const haschanged = async (currentUrl, tries = 1) => {
2121
if (currentUrl !== window.location.href) return
2222
return haschanged(currentUrl, ++tries)
2323
}
24+
25+
export const windowHistoryGo = (n) =>
26+
new Promise((resolve, reject) => {
27+
let timeout
28+
29+
// Resolve when the resulting popstate event happens
30+
const onPopState = () => {
31+
clearTimeout(timeout)
32+
window.removeEventListener('popstate', onPopState)
33+
resolve()
34+
}
35+
window.addEventListener('popstate', onPopState)
36+
37+
// Trigger the real go function
38+
window.history.go(n)
39+
40+
// timeout after 1 second
41+
timeout = setTimeout(() => {
42+
window.removeEventListener('popstate', onPopState)
43+
reject()
44+
}, 1000)
45+
})

‎packages/integration-tests/__helpers__/createTest.js‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { applyMiddleware, createStore, combineReducers } from 'redux'
22
import {
33
get,
44
clear,
5+
getCurrentIndex,
56
} from '@respond-framework/rudy/history/utils/sessionStorage'
67
import { locationToUrl } from '@respond-framework/rudy/utils'
78
import { createRouter } from '@respond-framework/rudy'
@@ -181,9 +182,12 @@ const createSnipes = (testName, routesMap, initialPath, opts, callback) => {
181182
return snapChange(++defaultPrefix, res, store, history, opts)
182183
},
183184
snap: async (action, prefix = '') => {
184-
prefix = prefix || JSON.stringify(action) || 'function'
185+
prefix =
186+
prefix ||
187+
(action && (JSON.stringify(action) || 'function')) ||
188+
'<none>'
185189

186-
const res = await store.dispatch(action)
190+
const res = action ? await store.dispatch(action) : null
187191

188192
if (opts.snipesOnly) return res
189193

@@ -374,6 +378,7 @@ const snapChange = (prefix, res, store, history, opts = {}, initialState) => {
374378

375379
if (opts.testBrowser) {
376380
expectSessionStorage(prefix)
381+
expectHistoryState(prefix)
377382
expectWindowLocation(prefix)
378383
}
379384
}
@@ -401,6 +406,10 @@ const expectSessionStorage = (prefix) => {
401406
expect(get()).toMatchSnapshot(`${prefix} - sessionStorage`)
402407
}
403408

409+
const expectHistoryState = (prefix) => {
410+
expect(window.history.state).toMatchSnapshot(`${prefix} - historyState`)
411+
}
412+
404413
const expectWindowLocation = (prefix) => {
405414
expect(locationToUrl(window.location)).toMatchSnapshot(
406415
`${prefix} - windowLocation`,
@@ -476,8 +485,7 @@ const awaitPop = async (history, tries = 1) => {
476485

477486
export const resetBrowser = async () => {
478487
/* eslint-env browser */
479-
const storage = get()
480-
const index = storage && storage.index
488+
const index = getCurrentIndex()
481489

482490
if (index) {
483491
const delta = index * -1

‎packages/integration-tests/__tests__/integration/browser/actionsInCallbacks/__snapshots__/jump.js.snap‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`jump after enter: 1 - historyState 1`] = `
4+
Object {
5+
"id": "345678",
6+
"index": 0,
7+
}
8+
`;
9+
310
exports[`jump after enter: 1 - response 1`] = `
411
Object {
512
"basename": "",
@@ -110,7 +117,6 @@ Object {
110117
"345678",
111118
],
112119
],
113-
"index": 0,
114120
}
115121
`;
116122

@@ -210,6 +216,13 @@ exports[`jump after enter: 1 - title 1`] = `"FIRST"`;
210216

211217
exports[`jump after enter: 1 - windowLocation 1`] = `"/"`;
212218

219+
exports[`jump after enter: firstRoute - /first - 1 - historyState 1`] = `
220+
Object {
221+
"id": "@@rudy/345678",
222+
"index": 0,
223+
}
224+
`;
225+
213226
exports[`jump after enter: firstRoute - /first - 1 - initial_state 1`] = `
214227
Object {
215228
"location": Object {
@@ -341,7 +354,6 @@ Object {
341354
"345678",
342355
],
343356
],
344-
"index": 0,
345357
}
346358
`;
347359

@@ -411,6 +423,13 @@ exports[`jump after enter: firstRoute - /first - 1 - title 1`] = `"FIRST"`;
411423

412424
exports[`jump after enter: firstRoute - /first - 1 - windowLocation 1`] = `"/"`;
413425

426+
exports[`jump before enter: 1 - historyState 1`] = `
427+
Object {
428+
"id": "@@rudy/345678",
429+
"index": 0,
430+
}
431+
`;
432+
414433
exports[`jump before enter: 1 - response 1`] = `
415434
Object {
416435
"basename": "",
@@ -501,7 +520,6 @@ Object {
501520
"345678",
502521
],
503522
],
504-
"index": 0,
505523
}
506524
`;
507525

@@ -586,6 +604,13 @@ exports[`jump before enter: 1 - title 1`] = `"FIRST"`;
586604

587605
exports[`jump before enter: 1 - windowLocation 1`] = `"/"`;
588606

607+
exports[`jump before enter: firstRoute - /first - 1 - historyState 1`] = `
608+
Object {
609+
"id": "@@rudy/345678",
610+
"index": 0,
611+
}
612+
`;
613+
589614
exports[`jump before enter: firstRoute - /first - 1 - initial_state 1`] = `
590615
Object {
591616
"location": Object {
@@ -717,7 +742,6 @@ Object {
717742
"345678",
718743
],
719744
],
720-
"index": 0,
721745
}
722746
`;
723747

‎packages/integration-tests/__tests__/integration/browser/actionsInCallbacks/__snapshots__/jump2N.js.snap‎

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ exports[`jump(-2) after enter on load: 1 - routes - REDIRECTED - onComplete 1`]
66

77
exports[`jump(-2) after enter on load: 1 - routes - THIRD - thunk 1`] = `1`;
88

9+
exports[`jump(-2) after enter on load: firstRoute - /first - 1 - historyState 1`] = `
10+
Object {
11+
"id": "345678",
12+
"index": 0,
13+
}
14+
`;
15+
916
exports[`jump(-2) after enter on load: firstRoute - /first - 1 - initial_state 1`] = `
1017
Object {
1118
"location": Object {
@@ -222,7 +229,6 @@ Object {
222229
"345678",
223230
],
224231
],
225-
"index": 0,
226232
}
227233
`;
228234

@@ -337,6 +343,13 @@ exports[`jump(-2) after enter on load: firstRoute - /first - 1 - title 1`] = `"F
337343

338344
exports[`jump(-2) after enter on load: firstRoute - /first - 1 - windowLocation 1`] = `"/"`;
339345

346+
exports[`jump(-2) after enter: 1 - historyState 1`] = `
347+
Object {
348+
"id": "345678",
349+
"index": 1,
350+
}
351+
`;
352+
340353
exports[`jump(-2) after enter: 1 - response 1`] = `
341354
Object {
342355
"basename": "",
@@ -487,7 +500,6 @@ Object {
487500
"345678",
488501
],
489502
],
490-
"index": 1,
491503
}
492504
`;
493505

@@ -617,6 +629,13 @@ exports[`jump(-2) after enter: 1 - title 1`] = `"SECOND"`;
617629

618630
exports[`jump(-2) after enter: 1 - windowLocation 1`] = `"/second"`;
619631

632+
exports[`jump(-2) after enter: firstRoute - /first - 1 - historyState 1`] = `
633+
Object {
634+
"id": "345678",
635+
"index": 2,
636+
}
637+
`;
638+
620639
exports[`jump(-2) after enter: firstRoute - /first - 1 - initial_state 1`] = `
621640
Object {
622641
"location": Object {
@@ -818,7 +837,6 @@ Object {
818837
"345678",
819838
],
820839
],
821-
"index": 2,
822840
}
823841
`;
824842

@@ -924,6 +942,13 @@ exports[`jump(-2) before enter on load: 1 - routes - REDIRECTED - onComplete 1`]
924942

925943
exports[`jump(-2) before enter on load: 1 - routes - THIRD - beforeEnter 1`] = `1`;
926944

945+
exports[`jump(-2) before enter on load: firstRoute - /first - 1 - historyState 1`] = `
946+
Object {
947+
"id": "345678",
948+
"index": 0,
949+
}
950+
`;
951+
927952
exports[`jump(-2) before enter on load: firstRoute - /first - 1 - initial_state 1`] = `
928953
Object {
929954
"location": Object {
@@ -1140,7 +1165,6 @@ Object {
11401165
"345678",
11411166
],
11421167
],
1143-
"index": 0,
11441168
}
11451169
`;
11461170

@@ -1255,6 +1279,13 @@ exports[`jump(-2) before enter on load: firstRoute - /first - 1 - title 1`] = `"
12551279

12561280
exports[`jump(-2) before enter on load: firstRoute - /first - 1 - windowLocation 1`] = `"/"`;
12571281

1282+
exports[`jump(-2) before enter: 1 - historyState 1`] = `
1283+
Object {
1284+
"id": "345678",
1285+
"index": 0,
1286+
}
1287+
`;
1288+
12581289
exports[`jump(-2) before enter: 1 - response 1`] = `
12591290
Object {
12601291
"basename": "",
@@ -1385,7 +1416,6 @@ Object {
13851416
"345678",
13861417
],
13871418
],
1388-
"index": 0,
13891419
}
13901420
`;
13911421

@@ -1500,6 +1530,13 @@ exports[`jump(-2) before enter: 1 - title 1`] = `"FIRST"`;
15001530

15011531
exports[`jump(-2) before enter: 1 - windowLocation 1`] = `"/"`;
15021532

1533+
exports[`jump(-2) before enter: firstRoute - /first - 1 - historyState 1`] = `
1534+
Object {
1535+
"id": "345678",
1536+
"index": 2,
1537+
}
1538+
`;
1539+
15031540
exports[`jump(-2) before enter: firstRoute - /first - 1 - initial_state 1`] = `
15041541
Object {
15051542
"location": Object {
@@ -1701,7 +1738,6 @@ Object {
17011738
"345678",
17021739
],
17031740
],
1704-
"index": 2,
17051741
}
17061742
`;
17071743

@@ -1801,6 +1837,13 @@ exports[`jump(-2) before enter: firstRoute - /first - 1 - title 1`] = `"THIRD"`;
18011837

18021838
exports[`jump(-2) before enter: firstRoute - /first - 1 - windowLocation 1`] = `"/third"`;
18031839

1840+
exports[`jump(-2) in pathlessRoute: 1 - historyState 1`] = `
1841+
Object {
1842+
"id": "345678",
1843+
"index": 0,
1844+
}
1845+
`;
1846+
18041847
exports[`jump(-2) in pathlessRoute: 1 - response 1`] = `
18051848
Object {
18061849
"basename": "",
@@ -1916,7 +1959,6 @@ Object {
19161959
"345678",
19171960
],
19181961
],
1919-
"index": 0,
19201962
}
19211963
`;
19221964

@@ -2016,6 +2058,13 @@ exports[`jump(-2) in pathlessRoute: 1 - title 1`] = `"FIRST"`;
20162058

20172059
exports[`jump(-2) in pathlessRoute: 1 - windowLocation 1`] = `"/"`;
20182060

2061+
exports[`jump(-2) in pathlessRoute: firstRoute - /first - 1 - historyState 1`] = `
2062+
Object {
2063+
"id": "345678",
2064+
"index": 2,
2065+
}
2066+
`;
2067+
20192068
exports[`jump(-2) in pathlessRoute: firstRoute - /first - 1 - initial_state 1`] = `
20202069
Object {
20212070
"location": Object {
@@ -2217,7 +2266,6 @@ Object {
22172266
"345678",
22182267
],
22192268
],
2220-
"index": 2,
22212269
}
22222270
`;
22232271

0 commit comments

Comments
 (0)