{"action":"created","issue":{"url":"https://api.github.com/repos/CompuIves/codesandbox-client/issues/1158","repository_url":"https://api.github.com/repos/CompuIves/codesandbox-client","labels_url":"https://api.github.com/repos/CompuIves/codesandbox-client/issues/1158/labels{/name}","comments_url":"https://api.github.com/repos/CompuIves/codesandbox-client/issues/1158/comments","events_url":"https://api.github.com/repos/CompuIves/codesandbox-client/issues/1158/events","html_url":"https://github.com/CompuIves/codesandbox-client/pull/1158","id":366627534,"node_id":"MDExOlB1bGxSZXF1ZXN0MjIwMjUzNTU0","number":1158,"title":"Correctly update history for pop and replace actions","user":{"login":"pshrmn","id":1127037,"node_id":"MDQ6VXNlcjExMjcwMzc=","avatar_url":"https://avatars0.githubusercontent.com/u/1127037?v=4","gravatar_id":"","url":"https://api.github.com/users/pshrmn","html_url":"https://github.com/pshrmn","followers_url":"https://api.github.com/users/pshrmn/followers","following_url":"https://api.github.com/users/pshrmn/following{/other_user}","gists_url":"https://api.github.com/users/pshrmn/gists{/gist_id}","starred_url":"https://api.github.com/users/pshrmn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pshrmn/subscriptions","organizations_url":"https://api.github.com/users/pshrmn/orgs","repos_url":"https://api.github.com/users/pshrmn/repos","events_url":"https://api.github.com/users/pshrmn/events{/privacy}","received_events_url":"https://api.github.com/users/pshrmn/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-10-04T05:27:47Z","updated_at":"2018-10-09T15:06:54Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/CompuIves/codesandbox-client/pulls/1158","html_url":"https://github.com/CompuIves/codesandbox-client/pull/1158","diff_url":"https://github.com/CompuIves/codesandbox-client/pull/1158.diff","patch_url":"https://github.com/CompuIves/codesandbox-client/pull/1158.patch"},"body":"<!--\r\nPlease make sure you are familiar with and follow the instructions in the\r\ncontributing guidelines (found in the CONTRIBUTING.md file).\r\n\r\nPlease fill out the information below to expedite the review and (hopefully)\r\nmerge of your pull request!\r\n-->\r\n\r\n<!-- Is it a Bug fix, feature, docs update, ... -->\r\n**What kind of change does this PR introduce?**\r\n\r\nThis alters how the fake browser (`<Preview>`) manages its `history` state.\r\n\r\nFixes #1102.\r\n\r\n<!-- You can also link to an open issue here -->\r\n**What is the current behavior?**\r\n\r\nPreviously, all `urlchange` dispatches were treated as pushes (append URL to end of `history.state` and increment `state.historyPosition`). The fake browser's back and forward buttons got around this by manually updating the state, but manual `history.go()` and `history.replaceState()` calls resulted in an incorrect history array/position.\r\n\r\nThis behavior can be seen here:\r\n\r\nhttps://codesandbox.io/s/vqw1mnnl9y\r\n\r\n(steps to reproduce are described in #1102)\r\n\r\n<!-- if this is a feature change -->\r\n**What is the new behavior?**\r\n\r\nWhen `history.go()` is called, the `state.history` array is unchanged while the `state.historyPosition` is altered by the go amount.\r\n\r\nWhen `history.replaceState()` is called, the `state.history` array is altered to replace the current entry at `state.historyPosition` with the new URL.\r\n\r\n`BasePreview.handleForward()` and `BasePreview.handleBack()` no longer manually set state because they can rely on `commitUrl()` to handle this correctly.\r\n\r\n<!-- Have you done all of these things?  -->\r\n**Checklist**:\r\n<!-- add \"N/A\" to the end of each line that's irrelevant to your changes -->\r\n<!-- to check an item, place an \"x\" in the box like so: \"- [x] Documentation\" -->\r\n- [ ] Documentation (N/A?)\r\n- [ ] Tests (would probably be nice, but I'm not sure if there is a way to test this)\r\n- [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->\r\n- [ ] Added myself to contributors table (already in there) <!-- this is optional, see the contributing guidelines for instructions -->\r\n\r\n<!-- feel free to add additional comments -->\r\n\r\n<!-- Thank you for contributing! -->\r\n"},"comment":{"url":"https://api.github.com/repos/CompuIves/codesandbox-client/issues/comments/428229709","html_url":"https://github.com/CompuIves/codesandbox-client/pull/1158#issuecomment-428229709","issue_url":"https://api.github.com/repos/CompuIves/codesandbox-client/issues/1158","id":428229709,"node_id":"MDEyOklzc3VlQ29tbWVudDQyODIyOTcwOQ==","user":{"login":"pshrmn","id":1127037,"node_id":"MDQ6VXNlcjExMjcwMzc=","avatar_url":"https://avatars0.githubusercontent.com/u/1127037?v=4","gravatar_id":"","url":"https://api.github.com/users/pshrmn","html_url":"https://github.com/pshrmn","followers_url":"https://api.github.com/users/pshrmn/followers","following_url":"https://api.github.com/users/pshrmn/following{/other_user}","gists_url":"https://api.github.com/users/pshrmn/gists{/gist_id}","starred_url":"https://api.github.com/users/pshrmn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pshrmn/subscriptions","organizations_url":"https://api.github.com/users/pshrmn/orgs","repos_url":"https://api.github.com/users/pshrmn/repos","events_url":"https://api.github.com/users/pshrmn/events{/privacy}","received_events_url":"https://api.github.com/users/pshrmn/received_events","type":"User","site_admin":false},"created_at":"2018-10-09T15:06:54Z","updated_at":"2018-10-09T15:06:54Z","author_association":"CONTRIBUTOR","body":"Were you running into that bug immediately or when you had navigated to another page (e.g. `/1`) and then refreshed the preview?\r\n\r\nThere was a bug in the actual test sandbox (I just fixed it) that always assumed the initial value was 0 on load, so it broke if the pathname was a number (other than 0).\r\n\r\nThat conflicted with [this line](https://github.com/CompuIves/codesandbox-client/blob/4b74200d035e058d26d92e274abeee8311bb5a00/packages/sandbox-hooks/url-listeners.js#L18), which prevents you from pushing the same URL as the current location. I'm not actually sure that that behavior is ideal. While an app probably shouldn't push the same location as the current one, this introduces a different behavior than a non-sandboxed app would have.\r\n\r\nI don't want to touch that here in case I'm missing a reason for that check, but I believe that it can (should?) be removed."}}
{"actor":{"display_login":"pshrmn"}}