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

Stop serving the application manifest from /manifest.json #9424

Merged
merged 2 commits into from Dec 1, 2017

Conversation

hwillson
Copy link
Contributor

@hwillson hwillson commented Nov 28, 2017

As discussed in #6674, Meteor currently serves its own manifest file from /manifest.json. This location is not application configurable, and can conflict with other non-Meteor defined manifest files, that are already being served from this location. For example, Google's progressive web app documentation suggests using the /manifest.json location for its manifest. The newest version of the web app manifest spec recommends using a different location for the web app manifest, like

<link rel="manifest" href="/manifest.webmanifest">

but the /manifest.json location was recommended at one point. Regardless, there isn't really any reason why Meteor needs to use the /manifest.json location, so let's move it elsewhere.

This PR moves moves /manifest.json to /__meteor__/webapp/manifest.json.

A few extra notes:

  • It's worth noting that Meteor core doesn't appear to rely on or use the served /manifest.json file in any way. We could consider removing it, but as discussed in [1.3] webapp appears to force-serve manifest.json #6674, there are a few ways in which applications can leverage this information. Since we're already serving it up this PR assumes we're going to keep it, but let me know if you think we should drop it instead.

  • At some point we might want to consider serving all application metadata via a similar /__meteor__/[package name]/[file] endpoint. The bundle-visualizer is using _meteor/[package name]/stats for example - at some point we might want to consolidate this. For now though this PR is just addressing the issue raised in [1.3] webapp appears to force-serve manifest.json #6674 directly.

Fixes #6674.

Thanks!

Meteor currently serves its own manifest file from
`/manifest.json`. This location is not application
configurable, and can conflict with other non-Meteor
defined manifest files, that are already being served
from this location. There isn't really any reason why
Meteor needs to use the `/manifest.json` location, so
this commit moves it to `/__meteor__/webapp/manifest.json`.

Fixes meteor#6674.
@benjamn
Copy link
Contributor

benjamn commented Nov 28, 2017

This reminded me of #9407, which concerns the manifest used by appcache, but that uses /app.manifest instead of /manifest.json.

@benjamn benjamn added this to the Release 1.6.1 milestone Nov 29, 2017
@benjamn benjamn merged commit 93fe8f1 into meteor:devel Dec 1, 2017
@benjamn
Copy link
Contributor

benjamn commented Dec 1, 2017

Pushed these two commits to normalize other URLs with this style: 95d093a, 4aeb453

@Floriferous
Copy link
Contributor

Does this mean all apps should now have a directory called __meteor__/webapp/ with the manifest.json in it?

Not totally clear if this requires an action from developers (and if manifest.json actually works, since there has been some confusion in the past, including #6674 as already mentioned here), any best-practice to recommend to people stumbling on this?

@abernix
Copy link
Contributor

abernix commented Feb 27, 2018

@Floriferous The manifest.json that is being served is Meteor-specific, and used internally by Meteor. It's not to be confused with the so called "Web App Manifest" as specified here which I'm assuming you're referring to. It used to be served at /manifest.json, but is now served at /__browser/manifest.json. You shouldn't need to make any changes unless you were consuming this internal Meteor file yourself, in some manner.

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.

None yet

4 participants