Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vite] Update the Vite ping route in sw.ts when upgrading to Vite 3.0 #13826

Closed
vursen opened this issue May 20, 2022 · 4 comments · Fixed by #14099
Closed

[vite] Update the Vite ping route in sw.ts when upgrading to Vite 3.0 #13826

vursen opened this issue May 20, 2022 · 4 comments · Fixed by #14099
Labels
enhancement vite Tickets related to vite support

Comments

@vursen
Copy link
Contributor

vursen commented May 20, 2022

The ping route has changed in Vite 3.0 (vitejs/vite#6819), so we'll need to update it also in sw.ts once we get to upgrading. Otherwise, the URL can end up accidentally cached.

Before: /VAADIN/__vite_ping. After: /VAADIN/.

@vursen vursen added enhancement vite Tickets related to vite support labels May 20, 2022
@vursen vursen added this to To do in Frontend build optimization via automation May 20, 2022
@Artur-
Copy link
Member

Artur- commented Jun 27, 2022

Do you mean it now pings the base URL, i.e. /VAADIN/ and we should. make sure that this very path is never cached?

@Artur-
Copy link
Member

Artur- commented Jun 27, 2022

i.e. in sw.ts we have

  registerRoute(
    ({ url }) => url.pathname.startsWith(`${scopePath}VAADIN/__vite_ping`),
    networkOnly
  );

  registerRoute(
    ({ url }) => url.pathname.startsWith(`${scopePath}VAADIN/`),
    networkFirst
  );

So we should delete the first and make the second networkOnly?

@vursen
Copy link
Contributor Author

vursen commented Jun 27, 2022

So we should delete the first and make the second networkOnly?

Not exactly. I guess we should add a new networkOnly route that would only handle the base path and then we can remove the __vite_ping route.

@Artur-
Copy link
Member

Artur- commented Jun 27, 2022

Ah, sorry the second one was startsWith so we should just change the first one to be an exact match

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement vite Tickets related to vite support
Development

Successfully merging a pull request may close this issue.

2 participants