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

Remaining params with uri generator in v2? #291

Open
mikespub opened this issue Nov 2, 2024 · 3 comments
Open

Remaining params with uri generator in v2? #291

mikespub opened this issue Nov 2, 2024 · 3 comments

Comments

@mikespub
Copy link

mikespub commented Nov 2, 2024

Hi @lcobucci

continuing my exploration of uri generation with v2 beta1, there are a few things that are still making it difficult to use:

  1. all substitution values must be string - easily fixed with $params = array_map('strval', $params) but ok
  2. no rawurlencode() on substituted values - again, easily fixable up-front but see next point
  3. no idea which params have been used to generate the uri, and which are "left over" to handle ourselves (incl. encoding etc.)

That last point is the tricky one, because you would expect to:

  • have a way to get them back (pass substitutions by reference + remove them when consumed?), or
  • append them as query string by default (not always applicable)

The use case is that the application will prepare a bunch of parameters, some mandatory and others optional, and expect the uri generator to prepare an URL taking all of these into account for a particular route.
This is especially useful if you tweak the routes later, and therefore change which ones will end up as path params and which will remain as query params in each route.

Make sense?

@lcobucci
Copy link
Collaborator

lcobucci commented Nov 7, 2024

Hummm that's an interesting thing...

I have some thoughts on how to solve this but they might stretch a bit the definition of a "beta" release (e.g. changing the return type to provide more flexibility)

I might have some time tomorrow or Sunday to play with this a bit...

@mikespub
Copy link
Author

mikespub commented Nov 8, 2024

You could extend the forRoute() method with an optional 3rd argument to add any remaining params as query string, and default it to false.

That way, you don't need to break existing integrations, change return type or behavior by default, and it would fit the 2nd option above.

The 1st option above could be done semi-transparently too, but there would be side-effects if someone expects $substitutions to remain unchanged.

Or you could do both to support either option :-)

@lcobucci
Copy link
Collaborator

lcobucci commented Nov 8, 2024

I'm more inclined towards the idea of returning a Stringable object to encapsulate those complexities away - thus the minor BC-break in a beta version.

I don't expect that to be very impactful to people using the new api (only for people extending it).

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

2 participants