Skip to content

Commit 0530784

Browse files
bors[bot]bidoubiwa
andauthored
Merge #731
731: Fix double slash error and refactor api routes r=bidoubiwa a=bidoubiwa In this PR is linked to #652. This issue raises the problem where the requested URL has a double slash. This does not impact the usability of meilisearch-js but could potentially become a problem. Also the error handler shows clearly the double slash when a communication error occurs. To avoid this problem in the future this PR does the following: - REFACTO: Removed all starting slashes on sub-routes in the code - FIX: Add multiple check inside the http handler for the following cases: - If the pathname ends with a slash OR if the sub-route start with a slash, we do not add a slash - If the pathname ends with a slash AND if the sub-routes start with a slash, we remove the starting slash in the sub route - If the sub-routes ends with a slash, remove that slash for consistency in URL request (ex: you could have the following requests: `http://localhost:7700/indexes/` and `http://localhost:7700/stats` both url are not consistent in their trailing slash) - if the pathname ends with two slashes, remove one. - REFACTO: Center all routes at the start of both the Index and MeilISearch class. In order to have a better visibility on the different routes and to avoid having to scroll to find how a specific route was hard coded. Some design choices: - You might ask why I kept `const url = ...` instead of directly adding the result in the returned function. I chose that for readability. If you think it is a bad idea tell me so. - The static lists that are at the top of each class tries to not repeat itself. Maybe it should repeat itself for readibility purposes. If you think the way I made it is not readable feel free to tell me. Also if you think they should be in another file feel free to also tell me. This PR tests are failing ❌ because of an update on slash handling. Working tests are here #732. They need to be merged here for this PR to pass its tests. fixes: #731 #744 Co-authored-by: Charlotte Vermandel <[email protected]> Co-authored-by: cvermand <[email protected]>
2 parents 007d187 + 90236f7 commit 0530784

20 files changed

+1016
-106
lines changed

src/http-requests.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,22 @@ import {
77

88
class HttpRequests {
99
headers: {}
10-
url: string
10+
url: URL
1111

1212
constructor(config: Types.Config) {
1313
this.headers = {
1414
...(config.headers || {}),
1515
'Content-Type': 'application/json',
1616
...(config.apiKey ? { 'X-Meili-API-Key': config.apiKey } : {}),
1717
}
18-
this.url = config.host
18+
this.url = new URL(config.host)
19+
}
20+
21+
static addTrailingSlash(url: string): string {
22+
if (!url.endsWith('/')) {
23+
url += '/'
24+
}
25+
return url
1926
}
2027

2128
async request({
@@ -32,17 +39,14 @@ class HttpRequests {
3239
config?: Partial<Request>
3340
}) {
3441
try {
35-
const constructURL = new URL(this.url)
36-
constructURL.pathname = constructURL.pathname + url
37-
42+
const constructURL = new URL(url, this.url)
3843
if (params) {
3944
const queryParams = new URLSearchParams()
4045
Object.keys(params)
4146
.filter((x: string) => params[x] !== null)
4247
.map((x: string) => queryParams.set(x, params[x]))
4348
constructURL.search = queryParams.toString()
4449
}
45-
4650
const response: Response = await fetch(constructURL.toString(), {
4751
...config,
4852
method,

0 commit comments

Comments
 (0)