robisim74 / qwik-speak

Translate your Qwik apps into any language
https://robisim74.gitbook.io/qwik-speak/
MIT License
133 stars 15 forks source link

extractFromUrl API is awkward to use #118

Closed intellix closed 7 months ago

intellix commented 8 months ago

We've started using the qwikCity rewrites so english has no locale in the URL and other languages do like /pt-BR/somewhere.

I've noticed that when I use something like a route guard like so:

throw redirect(302, '/somewhere');

It doesn't include the locale, so I thought ok I've got to use the extracted locale so I'll use the extractFromUrl API from qwik-speak, but realised that all it does is a split by the first slash: https://github.com/robisim74/qwik-speak/blob/eed2147b6e3494a696b15f00e2109b66c70b3cd3/packages/qwik-speak/src/routing.ts#L261-L264

So if you have a route like /account/deposit then the extracted locale is "account" and you ALWAYS need to validate it first using that function: https://github.com/robisim74/qwik-speak/blob/eed2147b6e3494a696b15f00e2109b66c70b3cd3/packages/qwik-speak/src/routing.ts#L272-L274

So I feel like extractFromUrl doesn't work as you'd expect it to, it requires you to look at it's implementation to understand why it returns something different. I was expecting the extracted locale from the URL to already be valid basically and I've got to do this everywhere (or just export my own function):

export function inferLocaleFromUrl(url: string) {
  const localeMaybe = extractFromUrl(url);
  return validateLocale(localeMaybe) ? localeMaybe : defaultLocale;
}
robisim74 commented 8 months ago

@intellix I don't understand what the problem is:

export const onRequest: RequestHandler = ({ redirect, url }) => {
  let lang: string | undefined = undefined;
  const prefix = extractFromUrl(url);

  if (prefix && validateLocale(prefix)) {
    lang = config.supportedLocales.find(value => value.lang === prefix)?.lang;
  }

  const getPath = translatePath();
  throw redirect(302, getPath('/somewhere', lang || config.defaultLocale.lang));
};
intellix commented 8 months ago

I think because it's implied that you're extracting the locale from the URL, but in fact it just extracts the first segment which always requires the same additional work afterwards (validateLocale). Can't think of any reason not to always call validateLocale first.

Maybe a more useful export:

export function extractLocaleFromUrl(url: string) {
  const localeMaybe = extractFromUrl(url);
  return validateLocale(localeMaybe) ? localeMaybe : null;
}

Then it would look like:

Get locale using URL:

const locale = extractLocaleFromUrl(url) ?? config.defaultLocale.lang;

Get prefix for redirecting:

const locale = extractLocaleFromUrl(url);
const prefix = locale ? `/${locale}` : '';
throw redirect(302, `${prefix}/somewhere);
robisim74 commented 8 months ago

I initially created validateLocale because there were Qwik bugs that assigned the lang parameter the wrong value. It is used in the solution without url rewriting: https://github.com/robisim74/qwik-speak/blob/main/docs/tutorial-routing.md#routing

So a method was simply added to help extract the locale from the URL in the case of url rewriting.

If you want to make your own method that uses the two methods (which are used in two different solutions), just do it.

Then if you are using url rewriting, throw redirect(302, `${prefix}/somewhere); will not work, because the URL is not rewritten (the path is not translated), so you need to use:

throw redirect(302, getPath('/somewhere', lang || config.defaultLocale.lang));

robisim74 commented 7 months ago

The two functions will remain separate, because each has a different responsibility.

See also https://github.com/robisim74/qwik-speak/blob/main/docs/tutorial-routing-rewrite.md#routing