Ensures there is no unexpected slash in url before query params

[ygrishajev]

Fixes #2379

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 1883498:

Sandbox	Source
Vanilla	Configuration
Vanilla Typescript	Configuration
rsk-github-issues-example	Configuration
@examples-query-react/basic	Configuration
@examples-query-react/advanced	Configuration
@examples-action-listener/counter	Configuration

[netlify]

✅ Deploy Preview for redux-starter-kit-docs ready!

Name	Link
🔨 Latest commit	1883498
🔍 Latest deploy log	https://app.netlify.com/sites/redux-starter-kit-docs/deploys/62ff1ac4ac90a70008a50465
😎 Deploy Preview	https://deploy-preview-2470--redux-starter-kit-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[ygrishajev]

@phryneas @markerikson pls take a look if this one makes sense

[phryneas]

I'm gonna give it a thorough look in the evening :)

[phryneas]

I think now you went too far 😅

My point was really just about removing a trailing slash before a ? if the user manually added the trailing slash before.

The other behavior is what it is - and I think also quite "expected". We couldn't change that without a major release even if we wanted - just the small fix for #2379 could count as a "bugfix" release though.

[ygrishajev]

@phryneas sorry, I think I don't follow. Initial issue I opened is that there is a trailing slash appearing which is not added.

export const api = createApi({
  reducerPath: 'api',
  baseQuery: fetchBaseQuery({ baseUrl: '/api/foo' }),
  endpoints: (builder) => ({
    findAll: builder.query({
      query: (params) => ({ url: '', params }) // `url` is empty, so I expect the request to be sent to `/api/foo?$foo=bar`, not `/api/foo/?$foo=bar`
    })
  })
});

Current implementation is really adding a slash out of the blue. If trailing slashes are untouched it seems to me more explicit, obvious and up to a user how to handle that.

Anyway, if you provide an example to how you think this should work it'd be helpful.

[phryneas]

Phew... that is really hard to get over through text 😅
Your previous PR was perfectly fine except where it removed a trailing slash where the user had actively entered the trailing slash themselves.

So,

baseUrl	url	param	result
foo	bar		foo/bar
foo/	bar		foo/bar
foo	/bar		foo/bar
foo/	/bar		foo/bar
foo	bar	{ a: 1 }	foo/bar?a=1
foo	bar/	{ a: 1 }	foo/bar/?a=1
foo		{ a: 1 }	foo?a=1
foo/		{ a: 1 }	foo/?a=1

does that make it a bit more clear?

[ygrishajev]

Ok, yea, that makes more sense. Thanks

[ygrishajev]

ok @phryneas I think this should do it

[ibrahim-delice]

I have a base URL than ends with @.
In my endpoint, I give it a parameter which I imagine would look like: @foo, but the result is rather: @/foo.

Will this PR help me with my issue or should I create a new issue?

CC @phryneas
UPDATE:
See my comment below.

[ibrahim-delice]

Can we refactor this so it also handle cases where the base ends with @?

Example 1:

const delimeter = (base.endsWith('?') || base.endsWith('@')) ? '' : '/'

Example 2

let delimeter = '/'
if (base.endsWith('?') || base.endsWith('@')) {
  delimeter = ''
}

[phryneas]

This is still meant to work for normal http-style urls. If we added a myriad of symbols here, it would just become weirder and weirder. Do you maybe use too much of a string for the baseUrl? Usually you would already cut that off at the domain.

[ibrahim-delice]

This is still meant to work for normal http-style urls. If we added a myriad of symbols here, it would just become weirder and weirder. Do you maybe use too much of a string for the baseUrl? Usually you would already cut that off at the domain.

It's an external endpoint I'm using which I have no power of, unfortunately.
I have to append a dynamic value after @ and that's the reason the current solution doesn't work for my particular case.

I can make a workaround by hardcoding a random value - but that is not a long-term solution.

[phryneas]

Assume this is your url:

https://example.org/some/sub/path@foo

Then in your case, you would have to use a baseUrl of https://example.org/some/sub and have every endpoint be declared as path@foo, path@bar etc.

[ibrahim-delice]

That is another "hack" you can do, but still not a robust solution.

Assume this is your url:

https://example.org/some/sub/path@foo

Then in your case, you would have to use a baseUrl of https://example.org/some/sub and have every endpoint be declared as path@foo, path@bar etc.

path is and will always be a base-url, and not an endpoint. I do have to append on every endpoint which is nothing but code duplication.

This is why I think redux-toolkit should handle this scenario.

[phryneas]

Urls in the internet don't work like that. Joining urls or paths doesn't work like that. The path separator for urls is /.
If we add that @ here, it will cause bugs for others because it is not part of any standard. And five minutes down the road we will add even more symbols and cause even more bugs for even more people.

You won't get around adding that yourself or writing a helper for yourself - this is a non-standard scenario and we cannot support it.

[ibrahim-delice]

...five minutes down the road we will add even more symbols and cause even more bugs for even more people.

That's why I suggest having an optional method (or something) to customise the url based on the users need. That would solve everything now and later on.

[phryneas]

You can just handle that in a custom baseQuery:

const myCustomBase = 'http://foo.bar/asd@
const defaultBq = fetchBaseQuery({})
const myBaseQuery = (arg, ...rest) => {
  const newArg = typeof arg == 'string' ? myCustomBase + arg : { ...arg, url: myCustomBase+arg.url }
  return defaultBq(newArg, ...rest)
}

[phryneas]

I'm happy with the current behaviour, merging this now.