i18nRedirect - wrong logic on dynamic routes with both params - normal & query

[7iomka]

If I have site.com/en/catalog/5?brand=2page if I use i18nRedirect to fr language
instead of site.com/fr/catalog/5?brand=2
i go to site.com/fr/catalog/[id]?brand=1&id=6

Dynamic url structure for this page is [locale]/catalog/[id] with query parameter brand passed with Link;

[7iomka]

My current workaround (it's a bit hard)

const { query } = router;
  let newAsPath = router.pathname.replace('[locale]', locale);
  const { locale: localeParam, ...allParamsExceptLocale } = query;
  const realQueryParams = {};

  // eslint-disable-next-line array-callback-return
  Object.keys(allParamsExceptLocale).map((param) => {
    const paramValue = Array.isArray(allParamsExceptLocale[param])
      ? (allParamsExceptLocale[param] as Array<string>).join(',')
      : (allParamsExceptLocale[param] as string);
    if (router.pathname.includes(`[${param}]`)) {
      newAsPath = newAsPath.replace(`[${param}]`, paramValue);
      return;
    }
    realQueryParams[param] = paramValue;
  });

  const queryParamsString: string = !isEmpty(realQueryParams) ? `?${new URLSearchParams(realQueryParams)}` : '';

  const newUrl = newAsPath + queryParamsString;

  if (forcePageReload) {
    window.location.href = newUrl;
  } else {
    router.push(router.pathname, newUrl);
  }

[Vadorequest]

It may be related to #142, I noticed this issue today.

There was a recent PR about I18nLink at #136, I believe something may have gone wrong then. I'll try to fix it ASAP.

[samuelcastro]

Any updates on this?

[Vadorequest]

I was off those past 2 weeks and just got back today. This is still the most prioritized issue on NRN and I'll try to tackle it this week.

[Vadorequest]

I actually misunderstood this bug and thought it was something else. It's not as critical as I first thought of.

I added unit tests for the I18nLink component: #148 it's not yet merged.

@7iomka Could you post your code? Did you use params and query correctly? Because my tests are working fine.

    test('when using route params and query route params using nested paths', () => {
      const renderer = TestRenderer.create(<I18nLinkTest href={'/products/favourites/[id]'} params={{id: 5}} query={{userId: 1}} />);
      const link: any = renderer.toJSON();
      console.log(link)

      expect(link.type).toEqual('a');
      expect(link.children).toEqual(['Text']);
      expect(link.props.href).toEqual('/en/products/favourites/5?userId=1');
      expect(link).toMatchSnapshot();
    });

    test('when using route params and query route params using nested paths and forcing locale', () => {
      const renderer = TestRenderer.create(<I18nLinkTest href={'/products/favourites/[id]'} params={{id: 5}} query={{userId: 1}} locale={'fr'} />);
      const link: any = renderer.toJSON();
      console.log(link)

      expect(link.type).toEqual('a');
      expect(link.children).toEqual(['Text']);
      expect(link.props.href).toEqual('/fr/products/favourites/5?userId=1');
      expect(link).toMatchSnapshot();
    });

[Vadorequest]

@samuelcastro Where you also affected by this, or just checking up?

[7iomka]

@Vadorequest
Hi. I just called redirect when dynamic page, that use query params is opened

DEMO: https://www.youtube.com/watch?v=RITgtOkh76c

My final workaround that fixes:

export const i18nRedirect = (locale, router: NextRouter, forcePageReload = false): void => {
  const { query } = router;
  let newAsPath = router.pathname.replace('[locale]', locale);
  const { locale: localeParam, ...allParamsExceptLocale } = query;
  const realQueryParams = {};

  // eslint-disable-next-line array-callback-return
  Object.keys(allParamsExceptLocale).map((param) => {
    const paramValue = Array.isArray(allParamsExceptLocale[param])
      ? (allParamsExceptLocale[param] as Array<string>).join(',')
      : (allParamsExceptLocale[param] as string);
    if (router.pathname.includes(`[${param}]`)) {
      newAsPath = newAsPath.replace(`[${param}]`, paramValue);
      return;
    }
    realQueryParams[param] = paramValue;
  });

  const queryParamsString: string = !isEmpty(realQueryParams) ? `?${new URLSearchParams(realQueryParams)}` : '';

  const newUrl = newAsPath + queryParamsString;

  if (forcePageReload) {
    window.location.href = newUrl;
  } else {
    router.push(router.pathname, newUrl);
  }
};

[samuelcastro]

I'm just checking up, I haven't faced this problem yet.

[Vadorequest]

@7iomka Thanks for the demo. I understand the issue, but I'm not experiencing it myself. 🤔

Could you share the code that performs the language changes? As it's where the issue is coming from.

Nevertheless, I'm glad you worked it out. I'll try your change and see how it can be integrated by default.

[Vadorequest]

Closing this, I believe it's a misusage and not a bug from NRN.

An additional test case has been added to make sure this works as expected:
d31b346

[samuelcastro]

I think this is related: #301