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

Precache manifest missing entries when recompiled via webpack --watch #1790

Closed
lakesare opened this issue Dec 13, 2018 · 19 comments · Fixed by #2538
Closed

Precache manifest missing entries when recompiled via webpack --watch #1790

lakesare opened this issue Dec 13, 2018 · 19 comments · Fixed by #2538
Assignees
Labels
Bug An issue with our existing, production codebase. Developer Experience Related to ease of use for developers. workbox-webpack-plugin

Comments

@lakesare
Copy link

lakesare commented Dec 13, 2018

Library Affected:
workbox-webpack-plugin

Browser & Platform:
Webpack 2.3.3, workbox-webpack-plugin 3.6.3

Issue Description:
In the webpack --watch mode, precache-manifest.hash.js file at first generates a perfect list of files, but on subsequent updates it misses some files.
For example, files generated with CopyWebpackPlugin are missing (but including copyUnmodified: true in options helps).
Another kind of files that are missing, is fontawesome-webfont.eot?hash files (importes via this process https://medium.com/@chanonroy/webpack-2-and-font-awesome-icon-importing-59df3364f35c), this I don't yet know how to fix.

Here is a similar issue for another manifest plugin: shellscape/webpack-manifest-plugin#144.

@lakesare lakesare changed the title workbox-webpack-plugin workbox-webpack-plugin: in the webpack watch mode not all the files are put into manifest Dec 13, 2018
@lakesare lakesare changed the title workbox-webpack-plugin: in the webpack watch mode not all the files are put into manifest workbox-webpack-plugin: in the webpack watch mode not all files are put into manifest Dec 13, 2018
@jeffposnick jeffposnick added Bug An issue with our existing, production codebase. workbox-webpack-plugin labels Dec 17, 2018
@jeffposnick jeffposnick changed the title workbox-webpack-plugin: in the webpack watch mode not all files are put into manifest Precache manifest missing entries when recompiled via webpack --watch Dec 17, 2018
@arackaf
Copy link

arackaf commented Jan 5, 2019

Just adding to this, for completeness, the navigate fallback file is also ejected on re-build.

@jeffposnick
Copy link
Contributor

Sorry for letting this drop until now.

workboox-webpack-plugin background

workbox-webpack-plugin generates its precache manifest based on the contents of the current compilation's assets, as determined in a compiler hook into the emit event. I think that's fairly standard for a webpack plugin.

webpack --watch mode behavior

After running some local experiments with --watch mode, I've found that only brand new assets, and modifications to existing assets, show up in the compilation's assets available during each subsequent emit hook invocation.

I believe some other plugins (like webpack-manifest-plugin, as referenced in shellscape/webpack-manifest-plugin#144) maintain state information about all of the previous compilation's assets and make additions or changes to hashes to that state information upon each subsequent compilation. They then create their output based on the cumulative total of all the assets that have been created across multiple compilations. (This seems to be the purpose of the seed parameter that can be passed to webpack-manifest-plugin.)

Problems with cumulative state

workbox-webpack-plugin could do something similar, and generate its precache manifest based on a cumulative list of assets across multiple compilations.

The concern that I have, though, is that there is no way for a webpack plugin to detect that a given asset has been removed following a recompilation—the absence of an asset from any particular compilation can't be used as a signal that it was removed, since that scenario is identical to what would happen if it was still present but unchanged.

Unfortunately, it's pretty easy to trigger that scenario even without bringing any third-party plugins into the mix; if you're following the official guide for dynamic imports with output.chunkFilename: '[name].bundle.js', and you have code like:

// Assume this is in your main chunk:
async function load() {
  const b = await import(/* webpackChunkName: "b" */ './b.js');
  const c = await import(/* webpackChunkName: "c" */ './c.js');
}

If you remove the dynamic import for c in watch mode, that will trigger a recompilation, but the only entries provided in assets will be b and the main chunk. There's no indication that c is no longer emitted.

Possible solutions

Given that, it sounds like there's a tradeoff when in --watch mode between potentially leaving out manifest entries that should be there (the current behavior) and including entries that correspond to assets that have been deleted (if we maintained asset info across compilations, and cumulatively added to it).

Neither solution is ideal, but I think from a service worker functionality perspective, the current behavior might be preferable—if you include a URL in the precache manifest list that doesn't exist, the installation process of the updated service worker will end up failing entirely. That sounds worse than the current behavior, in which the updated service worker installs but precaches less than expected. I'd love to hear folks' thoughts, though.

Additional warnings

What I plan on doing regardless is to detect when workbox-webpack-plugin has been called across multiple compilations and add a warning to the webpack compilation letting developers know not to trust the output for anything that should be considered production-ready. I will include a link that points to this (hopefully complete) description of the problem and trade-offs to give developers more context. The ultimate recommendation is that developers should only deploy a service worker that was generated by a one-shot webpack compilation.

@arackaf
Copy link

arackaf commented Oct 3, 2019

Ugh Rock and a Hard place, Jeff. Sorry webpack leaves you so few good options.

Would plugging this specific bug make sense? Ie, on watch, manually, forcibly check for the navigateFallback, and just add that? Not perfect, but it would solve one of the worse manifestations of this bug.

@jeffposnick
Copy link
Contributor

So what I'd like to do for the v5 timeframe is add in the webpack compilation warning when we detect that re-compilation has occurred, letting folks know that the contents of the precache manifest might be inconsistent.

I'm going to explore a bit more to see if there's anything in the webpack internals that could give us the info we need about the complete set of assets, raise issues with the webpack maintainers if not, and in all likelihood switch things over to the cumulative asset manifest for the time being.

I don't want to block the v5 release on that work, as it's likely going to take some time. I feel like adding in the warning about inconsistently gives us some leeway to make this sort of change post-v5, since developers will have a heads-up about not relying on --watch for anything production-ready.

@jeffposnick jeffposnick removed this from the v5 milestone Oct 4, 2019
@jeffposnick jeffposnick added the Developer Experience Related to ease of use for developers. label Oct 4, 2019
@schummar
Copy link

Is there a recommendation how to use this in dev mode (with watch or dev server)?
Turning off the plugin then, obviously results in errors since the service worker is not available.
Leaving it on emits a lot of warnings - and the feeling that something might not work as expected.

@jeffposnick
Copy link
Contributor

There are a few approaches:

  • Serve a no-op, empty service-worker.js file from your development web server.
  • Put the call to navigator.serviceWorker.register() behind a process.env.NODE_ENV check.
  • If you actually do want aggressive precaching in your development environment (which, honestly, I don't think is very helpful), you might be able to cobble something together to work around this issue using manifestTransform in v5. I could imagine persisting the originalManifest parameter outside of the lifetime of a given compilation, and then re-adding any missing manifest entries each time a re-compilation occurs. You'd risk re-adding manifest entries for assets that have been deleted, though, but I guess it's a development environment and you could deal with some inaccuracy.

I'd personally go with one of the first two options, as I think testing your service worker is best done via a "staging" rather than live-reload "development" build.

@ezzatron
Copy link

I'd personally go with one of the first two options, as I think testing your service worker is best done via a "staging" rather than live-reload "development" build.

I don't know if it's just because our case is unusual, but this would be very painful for my team. We recently added a Workbox service worker to a legacy application, and getting it right was largely a process of trial and error. Our non-live-reload Webpack build times take minutes to complete, whereas live-reload lets us fail fast and move on.

Basically, sometimes what you're developing is the service worker itself, or requires the service worker to be present to function (e.g. Notifications on Android Chrome).

We also value having the service worker in live-reload situations because of the concept of dev/prod parity. If developers are always working without the service worker running, they will probably miss issues that can and do arise once a service worker is in play.

FWIW, we went with option 3 above, but it doesn't save us from getting spammed with redundant warning messages over and over:

Screen Shot 2019-11-15 at 9 46 46 am

P.S. I have some experience writing Webpack plugins, so I do sympathise with how challenging it could be to find a "real" fix for this. I just wanted to plead my case 🙏

@novaknole
Copy link

@jeffposnick any new solutions since then?

@jeffposnick
Copy link
Contributor

There aren't any new solutions at this time. The warning message that gets logged in v5 will hopefully let folks know about the issue while doing local development.

@devarthurribeiro
Copy link

My temporary solution. So I can make development lighter:

const withPWA = require('next-pwa');

const settings = {
  env: {
  },
  devIndicators: {
    autoPrerender: false,
  },
  pwa: {
    dest: 'public',
  },
};

module.exports = process.env.NODE_ENV === 'development' ? settings : withPWA(settings);

@greko6
Copy link

greko6 commented Apr 28, 2020

A similar approach as per @devarthurribeiro, but through the disable option:

const withPWA = require('next-pwa')
const prod = process.env.NODE_ENV === 'production'

module.exports = withPlugins([[withSaas], [withPWA], [withImages]], {
(...)
  pwa: {
    disable: prod ? false : true,
    dest: 'public'
  },
(...)

@jeffposnick
Copy link
Contributor

I don't think there's anything more we can do here, other than log the warning on recompilation that we've implemented.

@xharris
Copy link

xharris commented Sep 3, 2021

As specified in https://www.npmjs.com/package/next-pwa?activeTab=readme#configuration

You can also do:

const withPWA = require('next-pwa')
 
module.exports = withPWA({
  pwa: {
    disable: process.env.NODE_ENV === 'development',
    register: true,
    scope: '/app',
    sw: 'service-worker.js',
    //...
  }
})

I don't get it... Is there a solution that doesn't require using next.js? Or does next-pwa not require using next.js?

WilliamConnatser added a commit to runcitadel/ui that referenced this issue Nov 18, 2021
…e colors and theme, and initial app header design idea (#8)

* merge squashed feat/Next.js branch

* disabled service worked in dev environment due to GoogleChrome/workbox#1790

* added more components and edited theme

* Fix for native dependencies

* Faster docker builds I'm GitHub actions (#7)

Closes #7

* Support window.location.origin if BASE_URL is not set (#6)

When this is launched, the base URL may be different depending on if the user accesses over Tor, citadel.local, or locally using the IP.
By not depending on the base URL from process.env, this is fixed.

* first pass at top of page design, added h5s and h6s to CSS reset, edited theme and typography/design page layout, added @bitcoin-design/bitcoin-icons-react

Co-authored-by: Aaron Dewes <[email protected]>
@klarstrup
Copy link

As specified in https://www.npmjs.com/package/next-pwa?activeTab=readme#configuration
You can also do:

const withPWA = require('next-pwa')
 
module.exports = withPWA({
  pwa: {
    disable: process.env.NODE_ENV === 'development',
    register: true,
    scope: '/app',
    sw: 'service-worker.js',
    //...
  }
})

I don't get it... Is there a solution that doesn't require using next.js? Or does next-pwa not require using next.js?

whatever your pipeline, simply don't use workbox when process.env.NODE_ENV === 'development' 😌

@lourd
Copy link

lourd commented Sep 9, 2022

My team has a case where we want to generate and register the service worker in development. This is because we have caching set up in our service-worker for some very large assets that are not a part of our webpack build, and we also want to be able to test and receive push notifications in local development.

If that's also your case, I figured out this hacky workaround:

const nextConfig = {
  webpack(config, options) {
    if (!options.isServer) {
      const workboxPlugin = new InjectManifest({
        swSrc: "./src/service-worker/index.ts",
        swDest: "../public/service-worker.js",
        // In dev, exclude everything.
        // This avoids irrelevant warnings about chunks being too large for caching.
        // In non-dev, use the default `exclude` option, don't override.
        ...(options.dev ? { exclude: [/./] } : undefined),
      })
      if (options.dev) {
        // Suppress the "InjectManifest has been called multiple times" warning by reaching into
        // the private properties of the plugin and making sure it never ends up in the state
        // where it makes that warning.
        // https://github.com/GoogleChrome/workbox/blob/v6/packages/workbox-webpack-plugin/src/inject-manifest.ts#L260-L282
        Object.defineProperty(workboxPlugin, "alreadyCalled", {
          get() {
            return false
          },
          set() {
            // do nothing; the internals try to set it to true, which then results in a warning
            // on the next run of webpack.
          },
        })
      }
      config.plugins.push(workboxPlugin)
    }
    return config
  }
}

You may also need to set self.__WB_DISABLE_DEV_LOGS = true in the service-worker source to suppress extraneous warnings in the browser console.

@craftcodelab
Copy link

My team has a case where we want to generate and register the service worker in development. This is because we have caching set up in our service-worker for some very large assets that are not a part of our webpack build, and we also want to be able to test and receive push notifications in local development.

If that's also your case, I figured out this hacky workaround:

const nextConfig = {
  webpack(config, options) {
    if (!options.isServer) {
      const workboxPlugin = new InjectManifest({
        swSrc: "./src/service-worker/index.ts",
        swDest: "../public/service-worker.js",
        // In dev, exclude everything.
        // This avoids irrelevant warnings about chunks being too large for caching.
        // In non-dev, use the default `exclude` option, don't override.
        ...(options.dev ? { exclude: [/./] } : undefined),
      })
      if (options.dev) {
        // Suppress the "InjectManifest has been called multiple times" warning by reaching into
        // the private properties of the plugin and making sure it never ends up in the state
        // where it makes that warning.
        // https://github.com/GoogleChrome/workbox/blob/v6/packages/workbox-webpack-plugin/src/inject-manifest.ts#L260-L282
        Object.defineProperty(workboxPlugin, "alreadyCalled", {
          get() {
            return false
          },
          set() {
            // do nothing; the internals try to set it to true, which then results in a warning
            // on the next run of webpack.
          },
        })
      }
      config.plugins.push(workboxPlugin)
    }
    return config
  }
}

You may also need to set self.__WB_DISABLE_DEV_LOGS = true in the service-worker source to suppress extraneous warnings in the browser console.

based on this workaround, i just added this to my webpack config file, and the warning gone <3

module.exports = (env) => {
	if (env !== 'production') {
		Object.defineProperty(workboxPlugin, 'alreadyCalled', {
			get() {
				return false
			},
			set() {}
		})
	}
	...
}

young-do added a commit to young-do/ditto that referenced this issue Jun 17, 2024
아래의 문구를 없애기 위해 개발시 비활성화

 ⚠ GenerateSW has been called multiple times, perhaps due to running webpack in --watch mode. The precache manifest generated after the first call may be inaccurate! Please see GoogleChrome/workbox#1790 for more information
AhmedGarcia added a commit to AhmedGarcia/TexEditPWA that referenced this issue Sep 15, 2024
….now im getting: InjectManifest has been called multiple times, perhaps due to running webpack in --watch mode. The precache manifest generated after the first call may be inaccurate! Please see GoogleChrome/workbox#1790 for more information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug An issue with our existing, production codebase. Developer Experience Related to ease of use for developers. workbox-webpack-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.