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

Rename params to slugs in load #3634

Comments

@Mlocik97
Copy link
Contributor

Mlocik97 commented Jan 30, 2022

Describe the problem

in load function we have instead of page now parameters url and params... params is derived from url.pathname, but url has also property url.searchParams... because of that it's kind of confusing as "params" can now refer to both parameters like ?x=value or to params like /[slug]/ ... I even twice was burned on that I did "const params = url.searchParams" in load function and rewrited params object from params of load function. (see, a lot of params word in this sentence)....

Describe the proposed solution

let url be url, and params be named slugs

Alternatives considered

No response

Importance

nice to have

Additional Information

No response

@Mlocik97 Mlocik97 changed the title Rename params to slug in load Rename params to slugs in load Jan 30, 2022
@Mlocik97 Mlocik97 changed the title Rename params to slugs in load Rename params to slugs in load Jan 30, 2022
@Mlocik97
Copy link
Contributor Author

Snímka obrazovky z 2022-01-30 17-57-34

as You can see, I'm not the only one confused about this. The name "params" for load "parameter" named "params", when it refer to slugs, while actuall "params" are avaiable in url.searchParams, is really bad...

@Conduitry
Copy link
Member

I had suggested slugs a month ago when we were reorganizing things to use the URL API, and Rich's reaction at the time was

eesh
i think we initially went with params because it has lineage (req.params in express and everything that copied it, for example)
i think slugs could be tricky because it's often the case that you have a paramater called slug
slugs.slug is an unpleasant thing to type

@Mlocik97
Copy link
Contributor Author

Mlocik97 commented Jan 30, 2022

interesting, and I agree with Rich that "slugs.slug" seems weird, but params seems as bad name as well... just today I found at least 4 people at Discord that were confused about this. We should think more about what name would be good there. maybe pathSlugs or dynamicPaths or idk (pngwn named it path params, so maybe pathParams?). Maybe I would even preferred slugs more than params, in this case, evne if "slugs.slug" is weird.

@didier
Copy link

didier commented Jan 31, 2022

request.values? request.entries? Just my two cents. I for some reason feel strongly for a single word variable name. Also, slugs sounds kinda gross. 😆

@Mlocik97
Copy link
Contributor Author

Mlocik97 commented Feb 1, 2022

actually we don't want to change request object as it's instance of Request, so it would be fine to have same... better to change params, not sure if entries would be good name for it... "values" is like, what is it? Not self-descripted...

@didier
Copy link

didier commented Feb 1, 2022

@Mlocik97 Oops, I didn't mean to say request. You’re right about it being less descriptive. What about parts, slices or sections?

@Mlocik97
Copy link
Contributor Author

Mlocik97 commented Feb 1, 2022

sections seems interesting suggestion, also parts but it's little less descriptive, like parts of what?,... about slices I think this is not good suggestion as there is slice function in JS, and if You would use both slice and slices in load it would be maybe confusing in some situations. Also it would be less readable to read similiar words, would need to make extra attention to last letter.

But I'm OK with sections

@didier
Copy link

didier commented Feb 1, 2022

Yeah, my thought process behind sections was that they are literal sections of the url. example.com/<section>/<section>, etc.

@Rich-Harris
Copy link
Member

Unless we can find a name that's unambiguously better than params, I think we probably need to keep it as-is. It was chosen because it matches req.params in Express apps, and is also used by Nuxt...

image

...and Remix:

image

It shouldn't be the overriding consideration, but there's value in using the same terminology as the wider ecosystem.

@Mlocik97 Mlocik97 closed this as completed Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment