Skip to content
This repository has been archived by the owner on Dec 26, 2023. It is now read-only.

Use webpack output path for icons src #23

Closed
dmtrKovalenko opened this issue Sep 4, 2017 · 8 comments
Closed

Use webpack output path for icons src #23

dmtrKovalenko opened this issue Sep 4, 2017 · 8 comments

Comments

@dmtrKovalenko
Copy link

dmtrKovalenko commented Sep 4, 2017

I got a trouble using manifest, because output manifest contains absoulute path, not relative to my output path.

  output: {
     publicPath: '/assets/'
  },
  new WebpackPwaManifest({
      background_color: '#ffffff',
      theme_color: '#2196f3',
      start_url: '/',
      icons: [
        {
          src: path.resolve(__dirname, '..', '..', '..', 'public', 'assets', 'icon.png'),
          sizes: [36, 48, 57, 72, 96, 114, 144, 192] // multiple sizes
        }
      ]
    })

manifest.json:

{
      "src": "assets/icon_192x192.7d461de01ff05a5ceb29aaca7ef5695f.png",
      "sizes": "192x192",
      "type": "image/png"
    },
    {
      "src": "assets/icon_144x144.8f737a65911f044a30e8ec64a14c6668.png",
      "sizes": "144x144",
      "type": "image/png"
    },
    {
      "src": "assets/icon_114x114.5ed529289e704528d188cf8798650827.png",
      "sizes": "114x114",
      "type": "image/png"
},
@arthurbergmz
Copy link
Owner

I'm sorry, but I didn't quite understand your problem.
What do you expect an output path? Can you show an example?

@dmtrKovalenko
Copy link
Author

dmtrKovalenko commented Sep 8, 2017

By default manifest is watching to public path, which is mywebsitedomain.com/assets. But when generated manifest is applying in such way (assets/icon..) icon resolves like mywebsitedomain.com/assets/assets/icon...
That can be fixed just if src will be generated with / at start
"src": "/assets/icon_144x144.8f737a65911f044a30e8ec64a14c6668.png"

@TemaSM
Copy link

TemaSM commented Sep 9, 2017

+1
Confirm this bug.
My publicPath is set to /wp-content/themes/demo/, webpack works fine, but pwa-manifest generates invalid links to icons.
Generated:

"icons": [
    {
      "src": "wp-content/themes/demo/public/img/icon_512x512.d66b2.png",
      "sizes": "512x512",
      "type": "image/png"
    }

But should be:

"icons": [
    {
      "src": "/wp-content/themes/demo/public/img/icon_512x512.d66b2.png",
      "sizes": "512x512",
      "type": "image/png"
    }

So, here is how manifest looks like in dev tools (empty icons, cause invalid path in src option):

image

I think the problem with conditional & substring at this line (tested it, confirmed):

return normalizeURI(join[0] === '/' ? join.substring(1) : join)

@arthurbergmz
Copy link
Owner

@TemaSM It's not the best solution, since it will affect #17 ...

I've been busy lately, but I'll be looking into it as soon as possible!

@TemaSM
Copy link

TemaSM commented Sep 11, 2017

@arthurbergmz, yeah, @kenrick95 noticed that, so I will push new commit to PR.
Probably the best solution will be using path module from Node.js core and it's functions:
path.join([...paths])
and/or
path.normalize(path)

@AleCaste
Copy link

AleCaste commented Sep 15, 2017

I was going to open an issue about this very same problem but I realized some people are getting the same problem too.
In our case the publicPath is /apps/mobile/ and we are getting the manifest injected in my html file like this (which is wrong):
<link rel="manifest" href="apps/mobile/manifest.615193c0a5c062ab4ee32ff4cc9d3019.json"/>
... it should be injected like this:
<link rel="manifest" href="/apps/mobile/manifest.615193c0a5c062ab4ee32ff4cc9d3019.json"/>
The icon files in the actual manifest file are also wrong, presenting the same problem.
Example:

   {
      "src": "apps/mobile/icon_144x144.ce8b6dcffad7a98b6c738fc856509188.png",
      "sizes": "144x144",
      "type": "image/png"
    }

Please note that in our case, the publicPath we are using does NOT really exist in the filesystem.
It is a publicPath that is used to later deploy the generated build files in another server machine.
What I am trying to say is that publicPath may sometimes refer to a location that does not necessarily exist on the machine where the build is being generated. It could be a deploy context used to mount the app on other servers.

Please let us know when this issue is fixed so that we can update our project.
Thanks a lot!

@arthurbergmz
Copy link
Owner

Please, update to v.3.3.2, it should be fixed now.
Can you guys, please, give me a feedback about it?

@arthurbergmz arthurbergmz removed the wip label Sep 18, 2017
@arthurbergmz
Copy link
Owner

Since no one else is talking about this issue after the update, I'm assuming that it's fixed.
Closed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants