Skip to content

Commit

Permalink
feat(routing): replace getLocation with getCurrentURL (#6477)
Browse files Browse the repository at this point in the history 
getCurrentURL is the alternative of getLocation in the history router. This is changed as it's possible to create a URL out of a string, but not a location. We also don't use any properties of the location that aren't also present in a URL.

[FX-3191]

BREAKING CHANGE: change from `getLocation` to `getCurrentURL`, and return a URL object.
BREAKING CHANGE: createURL createURL now receives `currentURL` instead of `location`
BREAKING CHANGE: parseURL createURL now receives `currentURL` instead of `location`
  • Loading branch information
@Haroenv
Haroenv committed Dec 20, 2024
1 parent 68ce8dd commit 52c68ad
Show file tree
Hide file tree
Showing 13 changed files with 103 additions and 119 deletions.
14 changes: 8 additions & 6 deletions examples/js/e-commerce-umd/src/routing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ const router = window.instantsearch.routers.history<RouteState>({
.join(' | ');
},

createURL({ qsModule, routeState, location }): string {
const { protocol, hostname, port = '', pathname, hash } = location;
createURL({ qsModule, routeState, currentURL }): string {
const { protocol, hostname, port = '', pathname, hash } = currentURL;
const portWithPrefix = port === '' ? '' : `:${port}`;
const urlParts = location.href.match(/^(.*?)\/search/);
const urlParts = currentURL.href.match(/^(.*?)\/search/);
const baseUrl =
(urlParts && urlParts[0]) ||
`${protocol}//${hostname}${portWithPrefix}${pathname}search`;
Expand Down Expand Up @@ -156,12 +156,14 @@ const router = window.instantsearch.routers.history<RouteState>({
return `${baseUrl}/${categoryPath}${queryString}${hash}`;
},

parseURL({ qsModule, location }): RouteState {
const pathnameMatches = location.pathname.match(/search\/(.*?)\/?$/);
parseURL({ qsModule, currentURL }): RouteState {
const pathnameMatches = currentURL.pathname.match(/search\/(.*?)\/?$/);
const category = getCategoryName(
(pathnameMatches && pathnameMatches[1]) || ''
);
const queryParameters = qsModule.parse(location.search.slice(1));
const queryParameters = qsModule.parse(currentURL.search, {
ignoreQueryPrefix: true,
});
const {
query = '',
page = 1,
Expand Down
14 changes: 8 additions & 6 deletions examples/js/e-commerce/src/routing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ const router = historyRouter<RouteState>({
.join(' | ');
},

createURL({ qsModule, routeState, location }): string {
const { protocol, hostname, port = '', pathname, hash } = location;
createURL({ qsModule, routeState, currentURL }): string {
const { protocol, hostname, port = '', pathname, hash } = currentURL;
const portWithPrefix = port === '' ? '' : `:${port}`;
const urlParts = location.href.match(/^(.*?)\/search/);
const urlParts = currentURL.href.match(/^(.*?)\/search/);
const baseUrl =
(urlParts && urlParts[0]) ||
`${protocol}//${hostname}${portWithPrefix}${pathname}search`;
Expand Down Expand Up @@ -158,12 +158,14 @@ const router = historyRouter<RouteState>({
return `${baseUrl}/${categoryPath}${queryString}${hash}`;
},

parseURL({ qsModule, location }): RouteState {
const pathnameMatches = location.pathname.match(/search\/(.*?)\/?$/);
parseURL({ qsModule, currentURL }): RouteState {
const pathnameMatches = currentURL.pathname.match(/search\/(.*?)\/?$/);
const category = getCategoryName(
(pathnameMatches && pathnameMatches[1]) || ''
);
const queryParameters = qsModule.parse(location.search.slice(1));
const queryParameters = qsModule.parse(currentURL.search, {
ignoreQueryPrefix: true,
});
const {
query = '',
page = 1,
Expand Down
14 changes: 8 additions & 6 deletions examples/react/e-commerce/routing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ const router = historyRouter<RouteState>({
.join(' | ');
},

createURL({ qsModule, routeState, location }): string {
const { protocol, hostname, port = '', pathname, hash } = location;
createURL({ qsModule, routeState, currentURL }): string {
const { protocol, hostname, port = '', pathname, hash } = currentURL;
const portWithPrefix = port === '' ? '' : `:${port}`;
const urlParts = location.href.match(/^(.*?)\/search/);
const urlParts = currentURL.href.match(/^(.*?)\/search/);
const baseUrl =
(urlParts && urlParts[0]) ||
`${protocol}//${hostname}${portWithPrefix}${pathname}search`;
Expand Down Expand Up @@ -152,13 +152,15 @@ const router = historyRouter<RouteState>({
return `${baseUrl}/${categoryPath}${queryString}${hash}`;
},

parseURL({ qsModule, location }): RouteState {
const pathnameMatches = location.pathname.match(/search\/(.*?)\/?$/);
parseURL({ qsModule, currentURL }): RouteState {
const pathnameMatches = currentURL.pathname.match(/search\/(.*?)\/?$/);
const category = getCategoryName(
(pathnameMatches && pathnameMatches[1]) || ''
);

const queryParameters = qsModule.parse(location.search.slice(1));
const queryParameters = qsModule.parse(currentURL.search, {
ignoreQueryPrefix: true,
});
const {
query = '',
page = 1,
Expand Down
8 changes: 4 additions & 4 deletions examples/react/ssr/src/App.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ function Hit({ hit }) {
return <Highlight hit={hit} attribute="name" />;
}

function App({ serverState, location }) {
function App({ serverState, currentURL }) {
return (
<InstantSearchSSRProvider {...serverState}>
<InstantSearch
Expand All @@ -31,12 +31,12 @@ function App({ serverState, location }) {
stateMapping: simple(),
router: history({
cleanUrlOnDispose: false,
getLocation() {
getCurrentURL() {
if (typeof window === 'undefined') {
return location;
return currentURL;
}

return window.location;
return new URL(window.location.href);
},
}),
}}
Expand Down
6 changes: 3 additions & 3 deletions examples/react/ssr/src/server.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ const app = express();
app.use('/assets', express.static(join(__dirname, 'assets')));

app.get('/', async (req, res) => {
const location = new URL(
const currentURL = new URL(
`${req.protocol}://${req.get('host')}${req.originalUrl}`
);
const serverState = await getServerState(<App location={location} />, {
const serverState = await getServerState(<App currentURL={currentURL} />, {
renderToString,
});
requestsCache.clear();
const html = renderToString(
<App serverState={serverState} location={location} />
<App serverState={serverState} currentURL={currentURL} />
);

res.send(
Expand Down
14 changes: 8 additions & 6 deletions examples/vue/e-commerce/src/routing.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,10 @@ const router = historyRouter({
.join(' | ');
},

createURL({ qsModule, routeState, location }) {
const { protocol, hostname, port = '', pathname, hash } = location;
createURL({ qsModule, routeState, currentURL }) {
const { protocol, hostname, port = '', pathname, hash } = currentURL;
const portWithPrefix = port === '' ? '' : `:${port}`;
const urlParts = location.href.match(/^(.*?)\/search/);
const urlParts = currentURL.href.match(/^(.*?)\/search/);
const baseUrl =
(urlParts && urlParts[0]) ||
`${protocol}//${hostname}${portWithPrefix}${pathname}search`;
Expand Down Expand Up @@ -163,12 +163,14 @@ const router = historyRouter({
return `${baseUrl}/${categoryPath}${queryString}${hash}`;
},

parseURL({ qsModule, location }) {
const pathnameMatches = location.pathname.match(/search\/(.*?)\/?$/);
parseURL({ qsModule, currentURL }) {
const pathnameMatches = currentURL.pathname.match(/search\/(.*?)\/?$/);
const category = getCategoryName(
(pathnameMatches && pathnameMatches[1]) || ''
);
const queryParameters = qsModule.parse(location.search.slice(1));
const queryParameters = qsModule.parse(currentURL.search, {
ignoreQueryPrefix: true,
});
const {
query = '',
page = 1,
Expand Down
4 changes: 2 additions & 2 deletions packages/instantsearch-core/src/__tests__/RoutingManager.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ describe('RoutingManager', () => {
// In a next refactor, we can consider changing this test implementation.
const parsedUrl = router.parseURL({
qsModule: qs,
location: window.location,
currentURL: new URL(window.location.href),
});

expect(parsedUrl.refinementList!.brand).toBeInstanceOf(Array);
Expand Down Expand Up @@ -627,7 +627,7 @@ describe('RoutingManager', () => {
// In a next refactor, we can consider changing this test implementation.
const parsedUrl = router.parseURL({
qsModule: qs,
location: window.location,
currentURL: new URL(window.location.href),
});

expect(parsedUrl.refinementList!.brand).toBeInstanceOf(Array);
Expand Down
58 changes: 22 additions & 36 deletions packages/instantsearch-core/src/routing/__tests__/historyRouter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,50 +80,50 @@ describe('life cycle', () => {
});
});

describe('getLocation', () => {
test('calls getLocation on windowTitle', () => {
const getLocation = jest.fn(() => window.location);
describe('getCurrentURL', () => {
test('calls getCurrentURL on windowTitle', () => {
const getCurrentURL = jest.fn(() => new URL(window.location.href));

historyRouter<UiState>({
windowTitle() {
return 'Search';
},
getLocation,
getCurrentURL,
cleanUrlOnDispose: true,
});

expect(getLocation).toHaveBeenCalledTimes(1);
expect(getCurrentURL).toHaveBeenCalledTimes(1);
});

test('calls getLocation on read', () => {
const getLocation = jest.fn(() => window.location);
test('calls getCurrentURL on read', () => {
const getCurrentURL = jest.fn(() => new URL(window.location.href));
const router = historyRouter<UiState>({
getLocation,
getCurrentURL,
cleanUrlOnDispose: true,
});

expect(getLocation).toHaveBeenCalledTimes(0);
expect(getCurrentURL).toHaveBeenCalledTimes(0);

router.write({ indexName: { query: 'query1' } });
jest.runAllTimers();

expect(getLocation).toHaveBeenCalledTimes(1);
expect(getCurrentURL).toHaveBeenCalledTimes(1);

router.read();

expect(getLocation).toHaveBeenCalledTimes(2);
expect(getCurrentURL).toHaveBeenCalledTimes(2);
});

test('calls getLocation on createURL', () => {
const getLocation = jest.fn(() => window.location);
test('calls getCurrentURL on createURL', () => {
const getCurrentURL = jest.fn(() => new URL(window.location.href));
const router = historyRouter<UiState>({
getLocation,
getCurrentURL,
cleanUrlOnDispose: true,
});

router.createURL({ indexName: { query: 'query1' } });

expect(getLocation).toHaveBeenCalledTimes(1);
expect(getCurrentURL).toHaveBeenCalledTimes(1);
});
});

Expand Down Expand Up @@ -188,25 +188,18 @@ describe('life cycle', () => {
search.start();
jest.runAllTimers();
}).toThrowErrorMatchingInlineSnapshot(
`"You need to provide \`getLocation\` to the \`history\` router in environments where \`window\` does not exist."`
`"You need to provide \`getCurrentURL\` to the \`history\` router in environments where \`window\` does not exist."`
);
});

test('does not fail on environments without window with provided getLocation', () => {
test('does not fail on environments without window with provided getCurrentURL', () => {
// @ts-expect-error
delete global.window;

expect(() => {
const router = historyRouter<UiState>({
getLocation() {
return {
protocol: '',
hostname: '',
port: '',
pathname: '',
hash: '',
search: '',
} as unknown as Location;
getCurrentURL() {
return new URL('http://localhost/');
},
cleanUrlOnDispose: true,
});
Expand All @@ -224,21 +217,14 @@ describe('life cycle', () => {
}).not.toThrow();
});

test('does not fail when running the whole router lifecycle with getLocation', () => {
test('does not fail when running the whole router lifecycle with getCurrentURL', () => {
// @ts-expect-error
delete global.window;

expect(() => {
const router = historyRouter<UiState>({
getLocation() {
return {
protocol: '',
hostname: '',
port: '',
pathname: '',
hash: '',
search: '',
} as unknown as Location;
getCurrentURL() {
return new URL('http://localhost/');
},
cleanUrlOnDispose: true,
});
Expand Down
Loading

0 comments on commit 52c68ad

Please sign in to comment.