robisim74 / qwik-speak

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

Text reverts back to default value on components containing modular-form #119

Closed genox closed 7 months ago

genox commented 8 months ago

Hi Roberto,

I noticed a strange behaviour that I can't explain and I wonder if you came across a similar event at some point and wether you could point me to the right direction.

I have a page that is composed from multiple conditional components (a checkout page in this case). Using the default locale (English) poses no issues, but when the locale is switched to a translation, on page refresh, I see the translated version of the text which is then immediately replaced by the original, English version. So to me it looks like the server is sending the correct string but the client part of qwik speak replaces it immediately after the page has been loaded.

Suspecting the auto keys feature, I disabled it and used a "regular" key with a translation counterpart on one element and disabled auto keys in vite.config.ts and tried again, but the issue persists even then.

https://github.com/robisim74/qwik-speak/assets/5611407/4147b3ce-a1aa-461d-81e3-943763d55468

The video shows what happens on one of the elements when refreshing the page.

It seems as a page increases in complexity, the more likely it is to run into this issue. I have no issues on CMS pages, but I just noticed the same thing on a product grid view.

robisim74 commented 8 months ago

the client part of qwik speak replaces it immediately after the page has been loaded

Are your dynamic components rendered server-side, or after the page becomes visible? Are they the ones who contain the overwritten texts? or the whole page indifferently?

I seem to notice some flickering from your video: you should see from the network tab if a redirect is being activated after the refresh which directs you to the page without the correct locale. In plugin.ts the onRequestmethod is generally responsible for setting the correct locale every time the page loads: perhaps another similar method is replacing it without setting the locale?

Are you using a localized url? if you navigate directly to the page with the locale in the URL, does it have the same problem?

If you are not using a localized URL, are you saving the locale in a cookie making it available to every navigation/refresh?

Did you setting base: extractBase in entry.ssr.tsx file?

robisim74 commented 8 months ago

Another thing: does it only happen in dev mode or also in prod mode?

genox commented 8 months ago

the client part of qwik speak replaces it immediately after the page has been loaded

Are your dynamic components rendered server-side, or after the page becomes visible? Are they the ones who contain the overwritten texts? or the whole page indifferently?

Some components are rendered server side and are being updated client side due to state updating itself on the form from an external source of state. The flicker you notice is basically the client side "render pass" I suppose.

I seem to notice some flickering from your video: you should see from the network tab if a redirect is being activated after the refresh which directs you to the page without the correct locale. In plugin.ts the onRequestmethod is generally responsible for setting the correct locale every time the page loads: perhaps another similar method is replacing it without setting the locale?

I can print the locale from one of the components and it is correct - on the server as well as on the client.

Are you using a localized url? if you navigate directly to the page with the locale in the URL, does it have the same problem?

I have

(in order of priority)

If you are not using a localized URL, are you saving the locale in a cookie making it available to every navigation/refresh?

Yes I set a cookie when the user changes the language manually

Did you setting base: extractBase in entry.ssr.tsx file?

Yes, this is present.

nelsonprsousa commented 8 months ago

I've been using qwik-speak for a while, not in production yet, and not a complex app yet, but didn't notice this problem.

Is it possible to provide a minimal reproducible example? Quite difficult to understand what's going on without it.

genox commented 8 months ago

Another thing: does it only happen in dev mode or also in prod mode?

This happens in dev mode as well as deployed in production mode.

I played around a bit more and I think it limits itself to views that have components that are being re-rendered. Either forced in code or based on user interaction (conditionally rendered etc).

The thing is, there are some other weird kinks that I experience currently that only started to show once a certain size and complexity of the app has been reached (multiple levels of nested components with a lot of state, etc).

Simple stuff always worked without an issue but suddenly more and more issues pop up that I essentially have no way of finding a root cause.

I will try to downgrade qwik later as I need to verify wether this happened before the last update (I was off for a month and I somehow remember not having this issue but my recollection might be spotty..)

genox commented 8 months ago

I've been using qwik-speak for a while, not in production yet, and not a complex app yet, but didn't notice this problem.

Is it possible to provide a minimal reproducible example? Quite difficult to understand what's going on without it.

If it is indeed the complexity that starts to show itself, then a repro will be difficult and I might as well share my repo with someone so you can see for yourself?

robisim74 commented 8 months ago

Some components [...] are being updated client side due to state updating itself on the form from an external source of state

It would be interesting to see one of these components: how it works and how it is updated.

Qwik's logic is: render the components server side, no hydration and resume the state. Do you have hydration?

Because following the logic of Qwik, this library uses useOnWindow('load') to resume its state (and make its context available client side ): that is, it makes it available when the entire page is now rendered and the external contents have been loaded (so as not to affect the first loading). Therefore, during a refresh, if you have hydration somewhere, it may happen that the library state is not yet available.

Maybe this is a limitation of this library, but what you are calling "complexity" of your app, maybe it's a sub-optimal way to use Qwik.

genox commented 8 months ago

Some components [...] are being updated client side due to state updating itself on the form from an external source of state

It would be interesting to see one of these components: how it works and how it is updated.

Qwik's logic is: render the components server side, no hydration and resume the state. Do you have hydration?

Because following the logic of Qwik, this library uses useOnWindow('load') to resume its state (and make its context available client side ): that is, it makes it available when the entire page is now rendered and the external contents have been loaded (so as not to affect the first loading). Therefore, during a refresh, if you have hydration somewhere, it may happen that the library state is not yet available.

Maybe this is a limitation of this library, but what you are calling "complexity" of your app, maybe it's a sub-optimal way to use Qwik.

This might very well be the case, I do not claim that this issue is with qwik-speak or with qwik itself, it might as well be my implementation that is causing this. This can merely be the symptom.

I will look into it further. Another similarity between the occurences is the usage of forms using modular forms on the same views and I just noticed that views like login and sign up forms also suffer from the same symptoms, which is something worth exploring.

On a side note - I would be sincerely thankful if I could gain another set of eyes that could give me some perspective of things that could be improved on the project i am currently working on. Unfortunately there is not much in it financially though. I am working on this solo, it's an ecommerce poc integrating medusaJS that I was planning to open source once the project is stable.

genox commented 8 months ago

This is indeed only happening in components that have some kind of form using modular-forms attached (which are many on an e-commerce oriented site, this possibility should have occurred to me earlier as only those seem to be affected).

As soon as <Form> is part of the markup, the entire component is re-rendered after the page loaded and a side effect might be that inlineTranslate functions used for labels etc. lose their context while that happens, reverting to default. This includes t() calls within or outside the <Form>.

I am trying to find a workaround.

genox commented 8 months ago

Here's a workaround:

When storing the translation result in a store and referencing the store value as label, the issue can be prevented:

  const labels = useStore({
    password: t('Password'),
  });
          <Field name="password">
            {(field, props) => (
              <TextInput {...props} {...field} type="password" label={labels.password} />
            )}
          </Field>

I will write an issue over at modular-forms. This is not a qwik-speak issue.

robisim74 commented 8 months ago

Oliver, I was able to recreate the problem:

        <Field name="password">
          {(field, props) => (
            <TextInput {...props} {...field} type="password" label={t('password')} />
          )}
        </Field>

The problem is the component TextInput, and the double spread props: {...props} {...field}.

If you look in the console, a chunks of the TextInput component is loaded which overrides the server-side rendering:

image

This shouldn't happen on the first load, and if I remove the double spread the problem no longer occurs:

<Field name="password">
  {(field, props) => (
    <TextInput {...props} field={field} type="password" label={t('password')} />
  )}
</Field>
export const TextInput = component$((props: any) => {
  return (
    <div>
      <p>{props.label}</p>
      <input {...props} type={props.type} value={props.value} />
      {props.field.error && <div>{props.field.error}</div>}
    </div>
  )
});

I would add, that even if React accepts multiple rests, Qwik shouldn't: as far as I know, it only accepts one ...rest, and personally I also think it's logical: I don't see any need to use a double spread in the props.

genox commented 8 months ago

Thank you for your solution and explanation. 👍

genox commented 7 months ago

@robisim74 Sorry for re-opening. I had some time to play around with this further.

While the above solution fixed the immediate issue on load, I now face another issue when submitting the form that reverts text back to the original language.

I tried to remove modular-forms from the equation and use useRouteAction and the original qwik Form component.

The form looks simply like this:

        <Form action={action}>
          <TextInputNormal name="email" value={''} error={''} type="email" label={t('E-Mail')} />
          <TextInputNormal
            name="password"
            value={''}
            error={''}
            type="password"
            label={t('Password')}
          />
          <div class="form-control mt-8 w-full max-w-xs">
            <button class={'btn btn-secondary'} type={'submit'}>
              {t('Sign in')}
            </button>
          </div>
        </Form>

The text input component is stripped of any references to modular forms and does not do any destructuring on component level:

type TextInputProps = {
  name: string;
  type: 'text' | 'email' | 'tel' | 'password' | 'url' | 'date';
  label?: string;
  placeholder?: string;
  value: string | undefined;
  error: string;
  required?: boolean;
};

export const TextInputNormal = component$(({ label, error, ...props }: TextInputProps) => {
  const { name, required } = props;
  return (
    <div class={'form-control mt-4 w-full'}>
      {label && (
        <label class="label" for={name}>
          <span class="label-text">
            {label} {required && <span>*</span>}
          </span>
        </label>
      )}
      <input
        name={name}
        value={props.value}
        type={props.type}
        placeholder={props.placeholder}
        id={name}
        aria-invalid={!!error}
        aria-errormessage={`${name}-error`}
        class="input input-bordered w-full"
      />
      {error && (
        <div id={`${name}-error`} class={'badge badge-info mt-3 text-sm'}>
          {error}
        </div>
      )}
    </div>
  );
});

The routeAction simply logs to console.

I think the initial issue is the same. On click, a bunch of code is loaded from the server that does not contain translations.

What I can see is that the resources requested do not contain a locale path segment, but that is ok on dev/localhost I assume?

I don't know what could be so special to be out of line with best practices here. This should simply work, no?

When using <Form reloadDocument={true} ... > the issue can be overcome as the page is being reloaded.

genox commented 7 months ago

This is how it looks like on the network tab.

https://github.com/robisim74/qwik-speak/assets/5611407/9b44861f-e3b7-4c83-b4f0-768851f877bc

genox commented 7 months ago

@robisim74 here's a repro: https://github.com/genox/qwik-form-issue-test

genox commented 7 months ago

Turns out this might be a qwik issue according to discussions on discord. Issue has been opened here: https://github.com/BuilderIO/qwik/issues/6045

robisim74 commented 7 months ago

Hey @genox , I understand that the incorrect language shows a problem that would otherwise remain hidden: but I can't solve app programming problems, and issues should be opened for library bugs, not apps.

That said, the issue is how your root.tsx is structured:

    <QwikCityProvider>
      <RootProviders>
        <ThemeInit defaultTheme={'dark'} />
        <head>

        </head>
        <body>

        </body>
      </RootProviders>
    </QwikCityProvider>

where:

export const RootProviders = component$(() => {
  return (
    <QwikCityProvider>
      <UiContextProvider>
            <Slot />
      </UiContextProvider>
    </QwikCityProvider>
  );
});

So you have QwikCityProvider twice, and you are not respecting the rule that QwikCityProvider should only have the head and body as children: https://qwik.builder.io/docs/api/#qwikcityprovider

Remove one <QwikCityProvider>, and remove <ThemeInit defaultTheme={'dark'} /> from above the head (you could bring it into the layout), and app will work again.

genox commented 7 months ago

@robisim74 I offer my sincerest apologies to have caused you to spend time on this and I understand the general rules. I got to a point where I was, quite frankly, desperate.

I can't thank you enough to point out my mistake. This would have been the last thing I would have verified. Let me know how I can compensate you for this, this literally saved my sanity. Since there is no easy way to give you a bottle of wine, let me know if you have a PayPal account for "wine by wire" and we pretend it's a Barolo. ;-)

robisim74 commented 7 months ago

Thanks for the offer of the wine, maybe we can drink it together sometime... but you are a contributor to this library, and you too have spent time.

Unfortunately, you've run into a software engineering problem that happens frequently: if the error isn't where you're looking for it, maybe you're looking for it in the wrong place.

Have a good evening!