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

vite-plugin-kit-routes: support anchor pages #799

Open
arkmech opened this issue Dec 17, 2024 · 14 comments
Open

vite-plugin-kit-routes: support anchor pages #799

arkmech opened this issue Dec 17, 2024 · 14 comments

Comments

@arkmech
Copy link

arkmech commented Dec 17, 2024

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

  1. I have hardcoded LINKS that are ENV specific.
  2. Support Anchor Pages example.com#section

Describe the solution you'd like

  1. Add ENV support for hardcoded LINKS
  2. Add a way to support Anchor pages

Describe alternatives you've considered

  1. Avoid using vite-plugin-kit-routes, and add in environment specific hardcoded links in in environment specific files env.local and env, lose type-safety.
  2. Template literal with ${route('/')}#
@jycouet
Copy link
Owner

jycouet commented Dec 17, 2024

Thx for reporting 👍
As there are 2 topics not really connected, could you split in 2 issues ? Can you also had an example for the env route ?
Ty

@arkmech
Copy link
Author

arkmech commented Dec 22, 2024

I have resolved:
1.vite.config.ts, supports ENV variables so this is all set.

So only issue is anchor page

@arkmech arkmech changed the title vite-plugin-kit-routes: support ENV specific routes and anchor links vite-plugin-kit-routes: support anchor links Dec 22, 2024
@arkmech arkmech changed the title vite-plugin-kit-routes: support anchor links vite-plugin-kit-routes: support anchor pages Dec 22, 2024
@jycouet
Copy link
Owner

jycouet commented Jan 7, 2025

I'm back :)

In vite.config.ts we could add a config anchors?

Something like:

kitRoutes({
  PAGES: {
    '/site': {
      anchors: '"part1" | "part2"'
    }
  }
})

To be used like:

route("/site", { __anchor: 'part2' })

Woudl this be good for you ?

@jycouet
Copy link
Owner

jycouet commented Jan 12, 2025

@arkmech any update ?

@arkmech
Copy link
Author

arkmech commented Jan 14, 2025

I'm back :)

In vite.config.ts we could add a config anchors?

Something like:

kitRoutes({
  PAGES: {
    '/site': {
      anchors: '"part1" | "part2"'
    }
  }
})

To be used like:

route("/site", { __anchor: 'part2' })

Woudl this be good for you ?

Seems like a valid api to me

@jycouet
Copy link
Owner

jycouet commented Jan 14, 2025

I started... and started to think about something else... Why not mixing it with explicit_search_params ?

kitRoutes({
  PAGES: {
    '/site': {
      explicit_search_params: { anchor: { type: '"section1" | "section2" | "section3"', required: true, isAnchor: true } },
    }
  }
})

It's not that nice to have it in searchParams section... But it would bring easily required, default, ....


Any hints ? Or you don't really care ?

@jycouet jycouet mentioned this issue Jan 14, 2025
@arkmech
Copy link
Author

arkmech commented Jan 15, 2025

Doesn’t matter, whatever makes sense the most sense in your implementation

@jycouet
Copy link
Owner

jycouet commented Jan 15, 2025

All right, would you test [email protected] ?

@OllieJT
Copy link

OllieJT commented Jan 19, 2025

I started... and started to think about something else... Why not mixing it with explicit_search_params ?

kitRoutes({
PAGES: {
'/site': {
explicit_search_params: { anchor: { type: '"section1" | "section2" | "section3"', required: true, isAnchor: true } },
}
}
})
It's not that nice to have it in searchParams section... But it would bring easily required, default, ....

Any hints ? Or you don't really care ?

Hey, just checking in here as this is also something I was looking for.

Excited for any solution to this as it's nice to have type safety for all routes.

With that said

  • an anchor is not a search param, and so this seems like an anti-pattern to me.
  • a url can only have one anchor, so the arrayMode on this would be defunct.

Here's what I'd recommend:

kitRoutes<KIT_ROUTES>({
   PAGES: {
      '/site': {
         // the URL class in js refers to this as a hash, but anchor would also be fine
         hash: {
            default: 'section1',
            type: '"section1" | "section2" | "section3"',
            required: true,
         },
      },
   },
})

@jycouet
Copy link
Owner

jycouet commented Jan 19, 2025

I agree with you @OllieJT 👍
The ony issue is that it's more code (as everything is already prepared for default, required, ...)
Also, not only for PAGES, but for LINK!

Did you check [email protected] ? Where you have the flag isAnchor ?


It's funny, you are saying:

Excited for any solution

But in the end, maybe not any solution ^^


I released under next.1 because also me I'm not sure about it... It was more a "quick win"!
Now thinking about it... Since all tests are already there, it's probably easier to switch to the right syntax.
(Also noted the hash, to be closer to the class 👍)

After all, it's all about time & motivation :D
Tonight? This week? month?
Give me strength 💪 ^^

@OllieJT
Copy link

OllieJT commented Jan 19, 2025

@jycouet I am excited for any solution ...but it looked like you were asking for feedback - so I shared my thoughts.

I'd love to jump in and help but I don't know when I'll have time at the moment. If you're open to contributions and I have time in the next week to learn how this works - I'd be happy to take a shot at it.

@jycouet
Copy link
Owner

jycouet commented Jan 19, 2025

Sweet!
Yes, thank you for the feedback. (Actually very happy about it... I was really at 50/50 (but my head was going your direction and my lazyness did the other thing haha))

I'm very open to contribution yes 😊
I can maybe look at it later, let me know if I should not to let you have a look this week ;)
If no news, I'll report back here 👍

@jycouet jycouet mentioned this issue Jan 19, 2025
@jycouet
Copy link
Owner

jycouet commented Jan 19, 2025

What do you think of: [email protected] ?
🙏 for the push 💪

@OllieJT
Copy link

OllieJT commented Jan 19, 2025

Commented on the PR to keep it in context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants