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

[workbox-webpack-plugin] v3 Entries array passed to manifestTransforms is empty #1227

Closed
akash5474 opened this issue Jan 25, 2018 · 23 comments
Closed

Comments

@akash5474
Copy link

akash5474 commented Jan 25, 2018

Library Affected:
workbox-webpack-plugin@3.0.0-alpha.6

Browser & Platform:
node v8.9.1, npm v5.5.1, Mac OSX 10.13.2

Issue or Feature Request Description:

Title describes the issue, the manifestEntries array passed as an argument into manifestTransform functions is empty.

I've created a repo to illustrate the problem. Here's a link to the branch with a minimal example reproducing the issue:

https://github.com/akash5474/workbox_ex/tree/manifestTransforms

@akash5474 akash5474 changed the title [workbox-webpack-plugin] Entries array passed to manifestTransforms is empty v3.0.0-alpha.6 [workbox-webpack-plugin] v3 Entries array passed to manifestTransforms is empty Jan 25, 2018
@gauntface
Copy link

My hunch is that your config is using chunks from webpack rather than globbing for any files from disk and it's only the globbed files that would appear in the manifestTransform.

@jeffposnick is this a case of docs needing updating?

@jeffposnick
Copy link
Contributor

@gauntface is correct. manifestTransforms applies to assets that are picked up outside of a webpack configuration.

I should be able to add in a new joi validation rule to detect when manifestTransforms is set and globPatterns isn't set, and throw a build-time error with more details.

This is covered by https://developers.google.com/web/tools/workbox/modules/workbox-webpack-plugin by virtue of manifestTransforms being listed under the "These options configure behavior unrelated to the webpack compilation." section, but that can obviously be overlooked.

@akash5474 if you have any feedback about improving the way that's documented (once you've had a look through that page), we're happy to reconsider.

@jeffposnick
Copy link
Contributor

...and just to explore some alternative approaches before we lock down the v3 interface:

  • We could apply manifestTransforms to the both the webpack-generated precache manifest and the precache manifest that's generated by file globbing. This may or may not be confusing for developers if there's one type of transformation that they expected only to apply to globbed files and it accidentally gets applied to entries from the webpack compilation.

  • We could create a separate configuration parameter, webpackManifestTransforms (or something like that) which specifically only applied the transformations to the manifest generated by the webpack compilation.

What's your use case for wanting to transform the precache manifest generated from the webpack assets, by the way? That might help inform the best path forward.

@akash5474
Copy link
Author

akash5474 commented Jan 26, 2018

@jeffposnick Thanks for the response and fixing the other issue I opened.

I did overlook the "These options configure behavior unrelated to the webpack compilation." text, completely my fault. A build time error would definitely be helpful.

Adding a gap between the tables might improve the visibility, I kind of scrolled past the row with that message without noticing (entirely my fault). Maybe adding a subsection in the right side menu underneath "GenerateSW Plugin" and "InjectManifest Plugin", though I don't know if that's a good solution.

Not sure my use case warrants changing the behavior of manifestTransforms or adding another parameter, but something like webpackManifestTransforms would probably work.

I'm building a single page app and was trying to use a transform to change the url for my html file (generated by html-webpack-plugin) from something like /static/app/index.html to /.

I want the revision for the precache-manifest entry corresponding to the html file (generated by html-webpack-plugin) to apply to the markup that I actually serve which is generated server-side (essentially the same html with some additional headers depending on the page and some other variables).

Based on available options, I think the correct configuration for my case (singlepage app and generating / server side) involves using templatedUrls. However templatedUrls doesn't seem to be ideal since the revision for / route mainly depends on /static/app/index.html file generated by webpack (injected assets with frequently updating hashes).

The server-side files will change much less often. I can't use a string and it would be difficult to use an array of glob patterns, I believe the pattern(s) would have to cover my entire app (and what if I update a dependency and my vendor file's hash changes?).

My config currently looks something like this:

module.exports = {
    entry: './src/index.js',

    output: {
        path: path.resolve(__dirname, 'dist/static/app'),
        publicPath: '/static/app/',
        filename: 'js/[name].js',
    },

    plugins: [
        new WorkboxPlugin.GenerateSW({
            swDest: '../../service-worker.js',
            navigateFallback: '/',
            templatedUrls: {
                '/': 'some-value'
            }
        })
    ]
};

Am I correct in thinking that the proper configuration for a single page app includes having bothnavigateFallback: '/' and an entry with url: '/' in the precache-manifest? It's possible I've missed something in my setup.

I've played around with different permutations of options and noticed if I don't have both of those, when I refresh the page after the service worker is registered I get a grey screen (see image below) even while online. I can only reload my app by clearing the service worker. This seems like it might be the correct behavior if navigateFallback: '/' is not set and no entry with url: '/' in the precache-manifest, but I'm not entirely sure since it results in the the page not loading.

workbox_error

@gauntface
Copy link

@akash5474 can you share what the set up on your project is, what the desirable manifest should be and what the actual manifest is.

At the moment I'm struggling to follow what is going wrong / options to fix it.

@jeffposnick
Copy link
Contributor

Am I correct in thinking that the proper configuration for a single page app includes having both navigateFallback: '/' and an entry with url: '/' in the precache-manifest? It's possible I've missed something in my setup.

Whatever URL you use for navigateFallback should also be listed in the precache manifest, yes. This is the URL that corresponds to your "app shell". If it's not cached, then you'd see the error you're getting in that screenshot (a discussion of this behavior is tracked in #1188).

If you're precaching your app shell via the URL /static/app/index.html, then have you tried specifying navigateFallback: '/static/app/index.html', instead of trying to manipulate the URLs via a transformation?

@akash5474
Copy link
Author

Whatever URL you use for navigateFallback should also be listed in the precache manifest, yes. This is the URL that corresponds to your "app shell".

@gauntface, I think @jeffposnick is on the right track to understanding my initial problem which led to me trying to use the manifestTransforms option incorrectly.

A better way to describe what I'm trying to accomplish might be that I'm generating part of the app shell server side. For example I may add some items to <head> based on the device type.

If you're precaching your app shell via the URL /static/app/index.html, then have you tried specifying navigateFallback: '/static/app/index.html', instead of trying to manipulate the URLs via a transformation?

I did try this but it caches the /static/app/index.html file which does not include the part of the app shell that is generated server side. I've added an example on another branch in the same repo which shows the problem (https://github.com/akash5474/workbox_ex/tree/reloadCantReach). This is the config I used

module.exports = {
    entry: './index.js',

    output: {
        path: path.resolve(__dirname, 'dist/static/app'),
        publicPath: '/static/app/',
        filename: 'js/[name].[hash].js',
    },

    plugins: [
        new HtmlWebpackPlugin({
            template: './index.template.html'
        }),
        
        new WorkboxPlugin.GenerateSW({
            swDest: '../../service-worker.js',
            navigateFallback: '/static/app/index.html',
        }),
    ]
}

And when using the templatedUrls option, the revision will not update when the hash of webpack's assets changes.

new WorkboxPlugin.GenerateSW({
    swDest: '../../service-worker.js',
    navigateFallback: '/',
    templatedUrls: {
        '/': ['./server.js']
    }
}),

@gauntface
Copy link

Is '/' made up of templates available in your webpack config? Or are these files only on the server.

@jeffposnick
Copy link
Contributor

I don't think you can use precaching for your HTML in that case. If you need to serve HTML that varies depending on some characteristic of the client's request headers, that's not nuance that you'd be able to represent with precaching.

You could use runtime caching instead, potentially with a stale-while-revalidate policy, and get some of the same benefits of taking network delays out of the rendering critical path after your initial navigation.

@gauntface
Copy link

You can also just add a server side generated manifest:

importScripts('/server-side-manifest.js');

workbox.precaching.precache(self.__serverSideManifest);

Then you would have a service side endpoint that would respond to /server-side-manifest.js and return something like:

self.__serverSideManifest = [
    {
       url: "/",
       revision: "........................"
    }
];

You would need to create a revision that is a hash of the contents of '/' (i.e. if you change the output of '/' the revision value changes too).

@jeffposnick
Copy link
Contributor

importScripts('/server-side-manifest.js');

won't work, because precaching relies on there being a change to the top-level service worker file in order to trigger a new install event. Nothing in the top-level service worker file wouldn't end up changing if there were a hardcoded '/server-side-manifest.js' URL.

(The webpack plugin, which also relies on an external manifest, automatically inserts a statement into the top-level service worker that looks like

importScripts('/precache-manifest.[hash].js');

That [hash] string value is updated whenever any of the contents of the precached URLs change.)

@gauntface
Copy link

@jeffposnick you are right is would need to be:

importScripts('/server-side-manifest.<revision>.js');

@raejin
Copy link

raejin commented Feb 8, 2018

How do you feel about supporting the templatedUrls option with the insight from Webpack compilation? Currently with our setup, I'm generating this piece of information outside of the Workbox plugin cycle as a workaround to generate hash for the shell since it depends on Webpack generated files. Because Webpack hasn't emit anything real to the disk at the plugin stage, globbing won't work.

Instead of utilizing globbing, perhaps we can support looking into compilation.assets and generate the hash based on the content?

@gauntface
Copy link

@raejin I can picture a scenario where that would be useful, not sure what the config options of that would look like however (and I'm a newb at Webpack). Could you open an issue with a proposal of a config that you think would fit this use case?

@ottoo
Copy link

ottoo commented Jun 9, 2018

It would be nice if manifestTransforms would be also applied to the webpack-generated precache manifest. We have a situation that the backend adds a prefix to some urls (but not all) so the generated precache manifest won't be correct.

For example the generated manifest would have /css/app.css, but our backend provides it as /v/[hash]/css/app.css for cache busting purposes (We don't have an index.html generated by webpack-html-plugin so automatic hashing doesn't work). If we added this prefix to each url, we could just use the webpack publicPath option, but with this configuration, it is not an option.

I think there would be an use-case for the before mentioned webpackManifestTransforms option. Maybe others have same kind of problems with this as well.

I could go around this by using workbox-build through a node script that is run after webpack build, but it would be nice to do this directly in the build step :)

@raejin
Copy link

raejin commented Jul 5, 2018

ditto to @ottoo's suggestion.

@jackyef
Copy link

jackyef commented Oct 9, 2018

Any updates regarding this?

Would love to see webpackManifestTransforms to be realized.

@jeffposnick
Copy link
Contributor

It's high on our list of planned changes for the workbox-webpack-plugin, but that work has unfortunately been delayed a bit as we get some new contributors up to speed.

The master tracking list for those updates is #1591

randytarampi added a commit to randytarampi/me that referenced this issue Oct 16, 2018
'cause the `Workbox` webpack plugin won't let you manually override paths to precached assets generated by webpack, per GoogleChrome/workbox#1227 (comment).
@manishbhatt94
Copy link

manishbhatt94 commented Oct 18, 2018

Hi @jeffposnick @gauntface..
I notice that adding manifest transformation to webpack generated assets is already on the roadmap for this project.
However I wanted to discuss my use case and wanted some suggestions on how to work around the same.

I'm using Next.js with workbox-webpack-plugin
Next emits the bundles / assets according to below folder structure:

client/build
├── bundles
│   └── pages
│       ├── _app.js
│       ├── _document.js
│       ├── _error.js
│       ├── index.js
├── chunks
│   ├── lazysizes_c7d1c726a13ef0281040919ab87052dd.js
├── precache-manifest.ea29d9a87efea6a22b482d6f07e9c0a8.js
├── static
│   ├── commons
│   │   ├── main.js
│   │   ├── manifest.js
│   ├── style.css
└── sw.js

Accordingly, the generated precache-manifest.[hash].js file is as:

self.__precacheManifest = [
  {
    "revision": "e17045946da6be83b2c8",
    "url": "bundles/pages/_app.js"
  },
  {
    "revision": "aafdbc9341182f758412",
    "url": "static/commons/main.js"
  },
  {
    "revision": "aafdbc9341182f758412",
    "url": "static/style.css"
  },
  {
    "revision": "01816bc9594df2c83999",
    "url": "static/commons/manifest.js"
  },
  {
    "revision": "7c5946399b2b21a5fc74",
    "url": "chunks/lazysizes_c7d1c726a13ef0281040919ab87052dd.js"
  },
  {
    "revision": "1e6a77ee5a8e6129f447",
    "url": "bundles/pages/index.js"
  },
  {
    "revision": "290487a965ccea8a6925",
    "url": "bundles/pages/_error.js"
  },
  {
    "revision": "e92680204ba0d06de76c",
    "url": "bundles/pages/_document.js"
  },
];

But Next.js serves above assets to very different paths, and so I need to transfrom the above manifest URLs to match the paths at which they are getting served by Next.js

For reference Next.js serves the emitted assets according to below sort of would-be modifyUrlPrefix mapping:

modifyUrlPrefix : {
  'bundles/pages/' : `/_next/${nextBuildId}/page/`,
  'chunks/'        : '/_next/webpack/chunks/',
  'static/'        : '/_next/static/',
},

My options currently AFAIK:

  1. Write Express GET routes for serving assets URLs as mentioned in precache-manifest.js and respond with the corresponding URLs served by Next.js - I agree this approach feels very wrong
  2. Somehow manually transform the generated precache-manifest.js file. - But I am not sure how to go about doing that
  3. Entirely exclude any webpack generated assets from landing in precache-manifest.js by providing an exclude/excludeChunks options, and instead picking them up by globPatterns so that I can then apply modifyUrlPrefix option on the generated precache-manifest.js - This might work (haven't tried yet) but I'm not exactly sure what disadvantages this approach might have or problems it might introduce

Kindly suggest how would you guys go about this problem!

@manishbhatt94
Copy link

manishbhatt94 commented Oct 30, 2018

Hi @jeffposnick @gauntface..
I notice that adding manifest transformation to webpack generated assets is already on the roadmap for this project.
However I wanted to discuss my use case and wanted some suggestions on how to work around the same.

I'm using Next.js with workbox-webpack-plugin
Next emits the bundles / assets according to below folder structure:

client/build
├── bundles
│   └── pages
│       ├── _app.js
│       ├── _document.js
│       ├── _error.js
│       ├── index.js
├── chunks
│   ├── lazysizes_c7d1c726a13ef0281040919ab87052dd.js
├── precache-manifest.ea29d9a87efea6a22b482d6f07e9c0a8.js
├── static
│   ├── commons
│   │   ├── main.js
│   │   ├── manifest.js
│   ├── style.css
└── sw.js

Accordingly, the generated precache-manifest.[hash].js file is as:

self.__precacheManifest = [
  {
    "revision": "e17045946da6be83b2c8",
    "url": "bundles/pages/_app.js"
  },
  {
    "revision": "aafdbc9341182f758412",
    "url": "static/commons/main.js"
  },
  {
    "revision": "aafdbc9341182f758412",
    "url": "static/style.css"
  },
  {
    "revision": "01816bc9594df2c83999",
    "url": "static/commons/manifest.js"
  },
  {
    "revision": "7c5946399b2b21a5fc74",
    "url": "chunks/lazysizes_c7d1c726a13ef0281040919ab87052dd.js"
  },
  {
    "revision": "1e6a77ee5a8e6129f447",
    "url": "bundles/pages/index.js"
  },
  {
    "revision": "290487a965ccea8a6925",
    "url": "bundles/pages/_error.js"
  },
  {
    "revision": "e92680204ba0d06de76c",
    "url": "bundles/pages/_document.js"
  },
];

But Next.js serves above assets to very different paths, and so I need to transfrom the above manifest URLs to match the paths at which they are getting served by Next.js

For reference Next.js serves the emitted assets according to below sort of would-be modifyUrlPrefix mapping:

modifyUrlPrefix : {
  'bundles/pages/' : `/_next/${nextBuildId}/page/`,
  'chunks/'        : '/_next/webpack/chunks/',
  'static/'        : '/_next/static/',
},

My options currently AFAIK:

  1. Write Express GET routes for serving assets URLs as mentioned in precache-manifest.js and respond with the corresponding URLs served by Next.js - I agree this approach feels very wrong
  2. Somehow manually transform the generated precache-manifest.js file. - But I am not sure how to go about doing that
  3. Entirely exclude any webpack generated assets from landing in precache-manifest.js by providing an exclude/excludeChunks options, and instead picking them up by globPatterns so that I can then apply modifyUrlPrefix option on the generated precache-manifest.js - This might work (haven't tried yet) but I'm not exactly sure what disadvantages this approach might have or problems it might introduce

Kindly suggest how would you guys go about this problem!

Hi.
So to solve my problem I went ahead with option 2 (i.e. manually transforming the generated precache-manifest.js file).

So I had to write a custom webpack plugin - very much similar to that used in next-offline

@manishbhatt94
Copy link

Guys sorry but I'm running into an issue with document (HTML) (pre)cache updation using Next.
If anyone has tried Next.js with Workbox and has come across similar problem, please help - or please provide any suggestion in general.
Linking detailed issue here: vercel/next.js#5553
Thanks 👍

@jeffposnick
Copy link
Contributor

Both modifyUrlPrefix and manifestTransforms will be supported in the Workbox webpack plugin v5, and will apply to assets created from the webpack compilation.

@jeffposnick
Copy link
Contributor

This should be addressed by the current Workbox v5.0.0 alpha.

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

No branches or pull requests

7 participants