Skip to content

Commit

Permalink
fix(history): avoid empty query string (#4130)
Browse files Browse the repository at this point in the history
**Summary**

This PR fixes an issue with the `historyRouter` + `simpleStateMapping`. They push an empty query string into the URL. The problem arise because we now always have an object with at least on key which is the top level index. The condition to add or not the query string is done on the `routeState` but it could be done on the query string itself since we already have computed it.

**Before**

![before](https://user-images.githubusercontent.com/6513513/65231669-2f1d1a80-dad0-11e9-8184-bd0ab3a8430d.gif)

**After**

![after](https://user-images.githubusercontent.com/6513513/65231680-32b0a180-dad0-11e9-9928-6a5231dfee60.gif)
  • Loading branch information
samouss authored Sep 19, 2019
1 parent 9605230 commit f6fd364
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
28 changes: 28 additions & 0 deletions src/lib/__tests__/RoutingManager-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -564,4 +564,32 @@ describe('RoutingManager', () => {
expect(parsedUrl.refinementList.brand).toBeInstanceOf(Array);
});
});

describe('createURL', () => {
it('returns an URL for a `routeState` with refinements', () => {
const router = historyRouter();
const actual = router.createURL({
query: 'iPhone',
page: 5,
});

expect(actual).toBe('https://website.com/?query=iPhone&page=5');
});

it('returns an URL for an empty `routeState` with index', () => {
const router = historyRouter();
const actual = router.createURL({
indexName: {},
});

expect(actual).toBe('https://website.com/');
});

it('returns an URL for an empty `routeState`', () => {
const router = historyRouter();
const actual = router.createURL({});

expect(actual).toBe('https://website.com/');
});
});
});
2 changes: 1 addition & 1 deletion src/lib/routers/history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const defaultCreateURL: CreateURL = ({ qsModule, routeState, location }) => {
const portWithPrefix = port === '' ? '' : `:${port}`;

// IE <= 11 has no proper `location.origin` so we cannot rely on it.
if (!routeState || Object.keys(routeState).length === 0) {
if (!queryString) {
return `${protocol}//${hostname}${portWithPrefix}${pathname}${hash}`;
}

Expand Down

0 comments on commit f6fd364

Please sign in to comment.