[✨] Reload page when QRLs return a 404 after code has changed and been redeployed

DustinJSilk commented 1 year ago

Is your feature request related to a problem?

When a production codebase is updated, the urls to QRL can change. This means that the website breaks for all users that are currently on the website.

Requests to fetch each chunk abort with a 404.

This can be tested by running a preview server, changing the code and restarting it, then interacting with the website.

Describe the solution you'd like

Since QRLs are serialised into the html, my guess is that we would need to completely refresh the page to update them. Unless there is a mechanism to refetch them.

It might be better to have an event/hook so that developers can decide what to do next when this happens.

Describe alternatives you've considered

We can't create aliases at build time because a user might have already loaded an out-of-date script which is incompatible with a newer version of another script.

This also didnt catch the aborted requests:

useOnWindow('error', () => {})

Additional context

This causes major issues with continuous integration.

gioboa commented 1 year ago

Hi @DustinJSilk, with the latest version of Qwik, is it still a valid issue?

DustinJSilk commented 1 year ago

Hi @gioboa, I havent tested it, is there a commit that has addressed the issue?

gioboa commented 1 year ago

In my production app I saw a refresh because of that. Now I'm trying to reproduce the case without success.

gioboa commented 8 months ago

This PR #5521 solves the issue. Thanks @DustinJSilk for reporting it.

DustinJSilk commented 6 months ago

@gioboa this still exists for me. The browser can still get stuck having the JS files cached and returning 404s until the site data is cleared and the page is refreshed. No amount of refreshing helps until the cache is emptied.

Can we please reopen this the website can get completely bricked by this issue.

This was tested on v1.4.5 which was released after that code change. i will monitor this with v1.5.3 going forward as well.

wmertens commented 6 months ago

That PR isn't sufficient, it's only for navigation, not for any other js loading.

wmertens commented 6 months ago

Note that 1.5.3 supports skew protection on vercel, which is another way of solving this, by making sure clients talk with a server that has their version.

Only works for 12 hours though, unless you pay more.

So the reload is still necessary.

DustinJSilk commented 6 months ago

Thanks for giving this your time, @wmertens !!

It seems like there are 2 issues at play here:

The docs state: Each build will create a new service worker, and the old service worker will be automatically unregistered. So if the page is refreshed and there is a new app, the service worker should get replaced and there shouldnt be any outdated code running. This didn't work for some reason until the user cleared all site data in the dev tools.
The other issue is when the app gets updated after the page has already loaded and then the QRLs start returning 404s or failing due to updates to the server side code.

Could we achieve something similar to Vercel Skew Protection (Im not on vercel unfortunately)

In some middleware we set a cookie with a site version, if a request arrives with an old version, we return a redirect to a page that can do a refresh. This should reregister the SW and clear the cache. Though as mentioned above it seems like there might have been another bug causing it to not work.

It doesn't seem very elegant and Im not sure if theres a better Qwik way of doing this.

PatrickJS commented 6 months ago

@DustinJSilk if you're using aws or anything with s3 to host the files you can deploy everything in /build folder in the same /build folder. this will fix the preview/next file version issue for files. For routeAction$ you can pass id string value as a second object to map to the same routeAction$ even when version changes. I'll add id config to server$ and see if we can add something in the framework for others to figure out the skew versions.

for vercel you have a deployment-id in a cookie then for each request you just map the deployment-id to the correct location. because its a cookie this will also attach to any assets but I mention above the fix is just deploying everything in /build in the same folder on a cdn since everything in /build is immutable

if the app was pwa then the version of the app will be downloaded (but still have mismatch action/loader/server$) https://github.com/QwikDev/pwa

tzdesign commented 5 months ago

@PatrickJS we had issues with action ids, even if the code does not change.

Can you point me to the location, where the hash is created? The ID is only available if validation is not given right?

Is the ID the shape of zod validation then?

PatrickJS commented 5 months ago

well it looks like if you have

{
  "id": "your-custom-action-id",
  "validation": zod({}),
}

I think @shairez requested this feature awhile back it looks like it was never documented

/** @public */
export const routeActionQrl = ((
  actionQrl: QRL<(form: JSONObject, event: RequestEventAction) => unknown>,
  ...rest: (CommonLoaderActionOptions | DataValidator)[]
) => {
  const { id, validators } = getValidators(rest, actionQrl);

  const getValidators = (rest: (CommonLoaderActionOptions | DataValidator)[], qrl: QRL) => {
  let id: string | undefined;
  const validators: DataValidator[] = [];
  if (rest.length === 1) {
    const options = rest[0];
    if (options && typeof options === 'object') {
      if ('validate' in options) {
        validators.push(options);
      } else {
        id = options.id;
        if (options.validation) {
          validators.push(...options.validation);
        }
      }
    }
  } else if (rest.length > 1) {
    validators.push(...(rest.filter((v) => !!v) as DataValidator[]));
  }

  if (typeof id === 'string') {
    if (isDev) {
      if (!/^[\w/.-]+$/.test(id)) {
        throw new Error(`Invalid id: ${id}, id can only contain [a-zA-Z0-9_.-]`);
      }
    }
    id = `id_${id}`;
  } else {
    id = qrl.getHash();
  }
  return {
    validators: validators.reverse(),
    id,
  };
};

tzdesign commented 5 months ago

Thanks @PatrickJS, will try it out tomorrow 🙏🙏🙏

gioboa commented 5 months ago

@tzdesign did you try this? is it working?

tzdesign commented 5 months ago

@gioboa I prepared qwik-insights for us in docker, as we are not allowed to send data to any other parties. (GDPR 😭)

But I will try this next week.

I saw that currently it's the qrl hash so it stays the same if you have no parameter changes.

The problem for us was, that we updated the cart object, which is a huge type.

We are not changing it so often, but I will try to fix it to a custom id. Because this action is more than crucial 😂

tzdesign commented 4 months ago

@PatrickJS looks like this is working:

{
  "id": "your-custom-action-id",
  "validation": [zod$({})],
}

Not sure why it needs to be an array. There is a signature with a type named REST.

Putting zod in plain, will fail typescript. But using it wrapped in an array works.

gioboa commented 1 month ago

is it solved?

DustinJSilk commented 1 month ago

No I wouldn’t say this is solved, not all hosting platforms support skew protection so it’s an issue that exists by default for Qwik apps

tzdesign commented 3 weeks ago

Yes, our shops run in kubernetes and every deployment we have tones of logs. My plan is to integrate onWindow next week. Because the page is somewhat frozen until reload.

QwikDev / qwik