Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[kit-routes] Support array search params #580

Closed
fehnomenal opened this issue Dec 28, 2023 · 7 comments · Fixed by #585
Closed

[kit-routes] Support array search params #580

fehnomenal opened this issue Dec 28, 2023 · 7 comments · Fixed by #585
Labels
enhancement New feature or request Kit Routes

Comments

@fehnomenal
Copy link
Contributor

Is your feature request related to a problem? Please describe.

I want to configure the following:

          explicit_search_params: {
            calculationIds: { type: 'number[]', required: true },
            workshopId: { type: 'string', required: true },
          },

but this fails in the generated file because appendSp converts all params to strings:

image

image

Describe the solution you'd like

appendSp should also accept string[] and number[] and append them. I guess something like this should work:

  const mapping = Object.entries(sp).flatMap(([name, val]) => {
    if (val == undefined) return [];
    if (Array.isArray(val)) return val.map((v) => [name, String(v)]);
    return [[name, String(val)]];
  });

Describe alternatives you've considered

Not setting the search params explicitely.

Additional context

Thank you very much for this plugin 🤗

@jycouet
Copy link
Owner

jycouet commented Dec 28, 2023

Sounds legit 😉
Will do it soon

@jycouet jycouet added enhancement New feature or request Kit Routes labels Dec 29, 2023
@jycouet
Copy link
Owner

jycouet commented Dec 29, 2023

Acutally, the only thing is to add types to the function, like https://github.com/jycouet/kitql/pull/583/files#diff-428c41f044580b0012cca0f191f227b016994088c35ee306a1ad47d70419ae23R186 No?

It's already adding a coma between values. (https://github.com/jycouet/kitql/pull/583/files#diff-33237037534b3bac096fcb3f8659e1b83232daf3002470f75d37f3f9d7104c07) So the mapping function is not needed.

I think that it's all good with this PR #583 but I want to make sure I understand all of it.

Please let me know before I merge and publish 👍

@jycouet
Copy link
Owner

jycouet commented Dec 29, 2023

@all-contributors please add @fehnomenal for example

Copy link
Contributor

@jycouet

I've put up a pull request to add @fehnomenal! 🎉

@fehnomenal
Copy link
Contributor Author

Acutally, the only thing is to add types to the function, like #583 (files) No?

It's already adding a coma between values. (#583 (files)) So the mapping function is not needed.

I think that it's all good with this PR #583 but I want to make sure I understand all of it.

Please let me know before I merge and publish 👍

Yeah you're right, it does not crash, I got misled by the type errors.

But there is a difference between ?ids=1&ids=2&ids=3 and ?ids=1%2C2%2C3. If I'd use this to generate internal links I could adapt the handling to the comma separated list. But I use this for external LINKS over which I have no control 😅

I opened #585 as an alternative approach. If you find it to complex just close it 😄

@jycouet
Copy link
Owner

jycouet commented Dec 30, 2023

OMG, I didn't know it was allowed to have multiple ids in SearchParams! 🤔
Is it common?

I like the idea of your PR by supporting multiple use-cases ✅

@jycouet
Copy link
Owner

jycouet commented Dec 31, 2023

Should be available in vite-plugin-kit-routes@0.3.0
Thx for your great work @fehnomenal 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Kit Routes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants