tighten / ziggy

Use your Laravel routes in JavaScript.
MIT License
3.91k stars 248 forks source link

Implement URI fragment handling #576

Closed daniser closed 1 year ago

daniser commented 2 years ago

Closes #551.

bakerkretzmar commented 2 years ago

@daniser can you give me permissions to push to this PR branch please?

bakerkretzmar commented 2 years ago

This is cool, thanks! I agree we should handle this more like Laravel's route helper does, I'm not sure yet how I feel about doing anything more than that.

I'm on board with sorting parameters that start with # to the end of the generated string, but I'm unsure about accepting and returning a new magic _fragment parameter. I can check as well, but do you know if Laravel supports retrieving the hash/anchor, like it does for the query string? I know there's $request->query(), is there $request->anchor() or something? Do you have a particular use case in mind for using the hash with route().params or route().current()?

Gonna keep thinking about this but I'm on board with the basic idea ๐Ÿ‘๐Ÿป

daniser commented 2 years ago

@daniser can you give me permissions to push to this PR branch please?

@bakerkretzmar No problem. Check it out, I think I done it right.

daniser commented 2 years ago

This is cool, thanks! I agree we should handle this more like Laravel's route helper does, I'm not sure yet how I feel about doing anything more than that.

I'm on board with sorting parameters that start with # to the end of the generated string, but I'm unsure about accepting and returning a new magic _fragment parameter. I can check as well, but do you know if Laravel supports retrieving the hash/anchor, like it does for the query string? I know there's $request->query(), is there $request->anchor() or something? Do you have a particular use case in mind for using the hash with route().params or route().current()?

Gonna keep thinking about this but I'm on board with the basic idea ๐Ÿ‘๐Ÿป

As for magic _fragment parameter, probably I've added it in order to level differences between ways of array handling in js and php. PHP allow mixing string and integer keys in its arrays, in JS it's not that... elegant? My first through on that was to use hash-prefixed keys when defining fragment using object syntax:

route('events.venues.show', {
    event: 1,
    venue: 2,
    '#details': true,
});

But, apart from using meaningless true value, it arose another ambiguity. Using multiple fragment definitions is non-standard AFAIK, tho you may have multiple "querystring"-like parameters within single fragment definition (check this Wikipedia article... well it uses fragment๐Ÿ˜Ž) - but it rarely used in practice, and I think it's outside of scope of this library.

As for your other thoughts, I'm afraid I can't answer them right now. Need to refresh my thoughts on this topic.

daniser commented 2 years ago

A quick search on Laravel codebase inside Illuminate\Routing namespace with keywords "fragment", "anchor", "hash" hasn't given any results except aforementioned RouteUrlGenerator::addQueryString method. As for route().current(). I have an UI tab system where each tab has corresponding fragment identifier (I think "anchor" is more HTML term). In this case route().current() can help me identifying which tab is currently open. This also answers why it is not needed in Laravel - "fragments" are more frontendish thing. URLs with them may be built on backend side, but its retrieval there has little to no use. Frontend use of fragments is much richer, I think.

bakerkretzmar commented 2 years ago

@daniser thanks. I think right now I'd like this to only move #-prefixed params to the end when it's safe to do that, nothing else. Can you update the PR or do you want me to?

The current tab thing is a compelling use case but I think it might be outside the scope of this package, and it's just as simple (and maybe even simpler) to check location.hash as it would be to check route().params or route().current().

daniser commented 2 years ago

@bakerkretzmar Ok, I'll do that. Should I keep _fragment magic key when passing parameters using object or do you have better alternatives in mind?

Tofandel commented 2 years ago

FYI, laravel can't retrieve the hash in the request, that's why it's not in there

It's all to do with the HTTP standard and browser security, the hash is not meant to be processed by the backend and the browser will never send it as a part of the url as per HTTP specs

https://stackoverflow.com/questions/3664257/why-is-the-hash-part-of-the-url-not-available-on-the-server-side#:~:text=The%20server%20can%20not%20read,passes%20it%20to%20the%20server.

Ziggy being a js router is not constrained by this limitation

bakerkretzmar commented 1 year ago

@daniser thanks a lot. I think I would rather leave the _fragment key out of this feature completely for now. Particularly since it is technically a breaking change for anyone already using a parameter called _fragment (highly unlikely but possible).

Can you update this PR again so that all it does is: sort parameter values starting with # to the end if the parameters are passed in as an array?

Sorry to keep chipping away at this feature, we can always incrementally add things later. Thanks!

bakerkretzmar commented 1 year ago

Closing this for now as Laravel's route() helper actually doesn't do this either, and I'm not sure we can get much closer to how they handle query params due to the JS/PHP array and object differences you mentioned. Thanks a lot for your work on this @daniser!

See https://github.com/tighten/ziggy/discussions/638#discussioncomment-5817186.