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

fix: support empty base in vite options #311

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nornagon
Copy link

vite supports an empty string for base, but if base: '' then VitePWA injects a manifest link to /manifest.webmanifest, instead of manifest.webmanifest. This fixes that :)

@netlify
Copy link

netlify bot commented Jun 23, 2022

Deploy Preview for vite-plugin-pwa ready!

Name Link
🔨 Latest commit 1d1fea4
🔍 Latest deploy log https://app.netlify.com/sites/vite-plugin-pwa/deploys/62b4ba487a319a00085d470b
😎 Deploy Preview https://deploy-preview-311--vite-plugin-pwa.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@userquin
Copy link
Member

@nornagon in Vite 2, using base with empty string, the result can be weird/indeterminate, you should use ./ instead; in Vite 3 you can use base : '' or base: './' to configure relative base

We'll keep in hold state...

thx

@userquin userquin requested a review from antfu July 8, 2022 08:28
antfu
antfu previously approved these changes Jul 8, 2022
@antfu antfu dismissed their stale review July 8, 2022 08:55

dismiss

@userquin userquin added the on hold more work to be done label Jul 10, 2022
@userquin
Copy link
Member

We will wait for the release of Vite 3, we need to review everything related to base, we are using it to set many defaults, such as scope and we are not sure yet if only including the changes in this PR will make the plugin work correctly.

@userquin
Copy link
Member

userquin commented Jul 12, 2022

To support Vite 3 Relative Base, we need to modify the code here to resolve the scope when registering the service worker, via the inline/registerSW/virtual scripts: for example, we can use document.baseURI, but this requires the user to modify the entry points after the app is deployed by adding <base>.

The webmanifest also has the above problem in that its scope cannot be changed once generated: of course the user must also modify the webmanifest once the app is deployed.

I think support for this can't be added here, it's a limitation on how the service worker works: if a user is going to deploy their application to a bunch of different base servers, they must be known and the user should create a build for each of them with the corresponding configuration, changing the base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold more work to be done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants