feat(i18n): better APIs

[ematipico]

Changes

This PR changes the APIs of the virtual module after some discussion in the RFC.

This PR changes the signature of the PRs, by passing a locale mandatory argument, and then a second argument: an object with a set of options.

This type of signature will allow us to add more options in the future.

The reason why this change was needed is that users might need:

• to add a path after the locale, without bothering with trailing slashes
• to add a path before the locale, without bothering with trailing slashes
• opt-in the normalisation of the locale, not always needed

Testing

I updated the tests

Docs

This time I need feedback on the APIs, especially the documentation. I will update the RFC accordingly later

[changeset-bot]

⚠️ No Changeset found

Latest commit: 1e41d6d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[natemoo-re]

Looking good! Left a few notes about comments that could be improved.

[martrapp]

LGTM!

[ElianCodes]

Looks good to me, but my gut feeling is to remove the 'it' from the start of the sentences.

[ElianCodes]

LGTM! but you might want to also want a @sarah11918 review.

[delucis]

Nice work @ematipico! I think the docs looks OK.

One thing I’m wondering is if this feels good to use in practice.

Thoughts:

• While there is the edge case of / where path can be omitted, most uses will provide a path (or will provide one programmatically, so may provide one even when the variable’s value is '/' or ''). That makes requiring { path: ... } feel a bit fiddly.

• What are the most common sources of data people might be feeding into these methods? Should they support things like getLocaleRelativeUrl('es', Astro.url)? (i.e. support a URL type as input, not just string.)

• The names are all get…Url(), but the return type is a string rather than a URL. Instead of separate relative and absolute methods could we have a single method, which lets you grab .pathname for relative or .href for absolute?

[TheOtterlord]

Docs looks good to me

+1 on the function naming & URL input support.

[ematipico]

While there is the edge case of / where path can be omitted, most uses will provide a path (or will provide one programmatically, so may provide one even when the variable’s value is '/' or ''). That makes requiring { path: ... } feel a bit fiddly.

I don't see any problem, we could change the signature of the API to (locale, path, options). I'll do that in another PR.

What are the most common sources of data people might be feeding into these methods? Should they support things like getLocaleRelativeUrl('es', Astro.url)? (i.e. support a URL type as input, not just string.)

The names are all get…Url(), but the return type is a string rather than a URL. Instead of separate relative and absolute methods could we have a single method, which lets you grab .pathname for relative or .href for absolute?

We could, but I am not sure about the flexibility of the APIs if we decided to have one single function and support for URLs. URLs work only with absolute URLs that have a protocol, and in many cases, users don't need it if they only need relative paths, or we don't have that information (site isn't provided).

Essentially, if the user needs a relative path, and we only have one API, it's technically impossible to return a URL (or use one internally).