Skip to content

fix: Use relative paths in manifest.json for compatibility with Github Pages #2271

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

Merged
merged 2 commits into from
Oct 12, 2018

Conversation

NoelDeMartin
Copy link
Contributor

When deploying applications using Github Pages, and other scenarios, absolute paths don't work because the website is served under a subdirectory. But using relative paths should work in all scenarios where manifest.json, index.html and the icons folder are on the same location. Which should be the case with the generated files if they are not modified.

In case this is not accepted, it should at least be documented because the current setup is generating PWAs that don't have proper icons in Github Pages. This same thing happens with vuepress, so once this is resolved I'll do the same on the other repo.

@Kocal
Copy link
Contributor

Kocal commented Aug 20, 2018

Looks good to me.
By the way what do you think about prefixing relative paths with ./? It would be more clear no?

@NoelDeMartin
Copy link
Contributor Author

@Kocal If you think that's better I don't mind. For web resources I am used to declare relative paths without the ./, but it's true that it is more explicit so I guess it's an unfunded habit (writing less I guess, which is no real justification).

Should I commit that change then? If you confirm it I'll go ahead.

@Kocal
Copy link
Contributor

Kocal commented Aug 20, 2018

Well it's just my own opinion, as you said it's more explicit (and Explicit is better than Implicit 👀).
If it's improve readability and if your fix is still working after that, so yes it would be fine to commit that change, thank you 🙂

@NoelDeMartin
Copy link
Contributor Author

@Kocal Ok done 👍

@haoqunjiang haoqunjiang merged commit 6d26c75 into vuejs:dev Oct 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants