Datx - required trailing slash in the current state

[Jokasxy]

Relevant version

• [ ] 0.x
• [ ] 1.x
• [X] 2.x

Relevant libraries

• [ ] utils
• [ ] core
• [X] network
• [ ] jsonapi
• [ ] jsonapi-angular

Breaking change

No

Description

In the current state of datx it is required that config.baseUrl contains a trailing slash.

If I'm correct this can be avoided by using a library like url-join or by adding a util function that normalizes url concatenation so it doesn't care if the trailing slash is provided.

I don't have deep knowledge about how the code works, but I think that the issue can be fixed by passing request._config.baseUrl and request._options.url in normalization function on line 110 in Network base request

[isBatak]

Just a thought. Is it necessary to use url-join lib when we now have native support with new URL https://developer.mozilla.org/en-US/docs/Web/API/URL/URL

const u = new URL('/todos', 'http://example.com/api/v1/');
u.toString()
// 'http://example.com/todos'

const u = new URL('/todos', 'http://example.com/api/v1')
u.toString()
// 'http://example.com/todos'

[isBatak]

@Jokasxy this line is the culprit https://github.com/infinum/datx/blob/master/packages/datx-jsonapi/src/helpers/url.ts#L158 I think if we wrapp it in new URL it will fix the problem. You can contribute if you have time.

@DarkoKukovec what do you think, am I right?

[Jokasxy]

@isBatak Like you have got there, you loose this "/api/v1" part like that so you would probably need to split the host from the rest in second param and append it before to the first param to use new URL

const path = 'tournament/1';
const apiURL = new URL(process.env.NEXT_PUBLIC_API_ENDPOINT as string);
const url = new URL(`${apiURL.href}${path.length <= 1 || path.startsWith('/') ? path : `/${path}`}`);
console.log(url.href);
// http://localhost:3000/api/v1/tournament/1
// same with /tournament/1
// path = '' gives http://localhost:3000/api/v1
// path = '/' gives http://localhost:3000/api/v1/

[DarkoKukovec]

Actually, it's this line: https://github.com/infinum/datx/blob/master/packages/datx-jsonapi/src/helpers/url.ts#L90

Is the issue that you have to add / here? It seems to me that it's fine to be explicit here, because otherwise we're assuming that / is always at the end, which might not be true?

[Jokasxy]

With this ${config.baseUrl}${url}, slash character needs to be added either at the end of {baseUrl} or at the start of the {url} e.g. public static readonly endpoint = '/tournaments';.

My idea adding a util function which will recognize how the url is formatted which could prevent zero and double slashes so we have a valid url anyhow.

[isBatak]

I agree with @Jokasxy double slashes should not happen. I had this issue before and I scratched my head for some time until I figured out what was the problem.

[DarkoKukovec]

I guessd we could try to prevent double slashes, but I'm njot convinced we should add slashes if they don't exist.

[isBatak]

I would like it to work with NEXT_PUBLIC_API_ENDPOINT=http://localhost:3000/api/v1 and NEXT_PUBLIC_API_ENDPOINT=http://localhost:3000/api/v1/

[isBatak]

Final conclusion was to implement predictable url building that avoids double shals issue. To opt-out from this decision endpoint should accept transformer function that receives baseUrl and returns newly constructed endpoint.

config.baseUrl = 'http://example.com/api';

class Example extends jsonapi(Model) {
  public static readonly endpoint = '_tournaments';
}
// endpoint: http://example.com/api/_tournaments

class Example extends jsonapi(Model) {
  public static readonly endpoint = (baseUrl: string) => `${baseUrl}_tournaments`;
}

// endpoint: http://example.com/api_tournaments