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

RFC: PWA Plugin #717

Closed
yyx990803 opened this issue Jan 10, 2018 · 29 comments
Closed

RFC: PWA Plugin #717

yyx990803 opened this issue Jan 10, 2018 · 29 comments
Labels

Comments

@yyx990803
Copy link
Member

/cc @addyosmani @jeffposnick @sudo-suhas

So, I've been working pretty hard on the next version of vue-cli, which uses a completely plugin-based architecture, and the PWA functionalities are now fully isolated in its own module: https://github.com/vuejs/vue-cli/tree/next/packages/%40vue/cli-plugin-pwa

A brief intro for the plugin API: index.js is the runtime plugin that will be loaded when users run npm run serve or npm run build. generator contains a module that will be invoked when the user scaffolds the project with the global vue command. The API should be pretty explanatory, and seems to cover most use cases. But any feedback is greatly appreciated.

Our previous PWA template is a fork of the main webpack template and has trouble keeping up with the upstream changes. This new setup should greatly reduce the maintenance scope for PWA-related features in the future.

After the launch of vue-cli 3.0, we should deprecate the pwa template and move all collaborations under the plugin instead.

@yyx990803 yyx990803 changed the title RFC: PWA RFC: PWA Plugin Jan 10, 2018
@sudo-suhas
Copy link
Contributor

sudo-suhas commented Jan 10, 2018

Hey @yyx990803, I wanted to test this out so I cloned the repo and tried to follow the steps in "Development Setup" from the readme but yarn link did not work correctly for me and the vue binary did not get installed globally. But I worked around that by doing this:

λ cd packages\test\

λ node ..\@vue\cli\bin\vue create test-app

However, after I manually selected the features, it failed to scaffold the project:

Vue CLI v3.0.0-alpha.1
✨  Creating project in E:\Projects\repos\vue-cli\packages\test\test-app.
🗃  Initializing git repository...
 ERROR  command failed: yarn add --dev @vue/cli-plugin-babel @vue/cli-plugin-eslint @vue/cli-plugin-unit-jest @vue/cli-plugin-pwa @vue/cli-service
error An unexpected error occurred: "http://registry.npmjs.org/@vue%2fcli-plugin-babel: Not found".

What do I need to do to make this work?

Another question I had was, what about the documentation? Would each plugin have it's own documentation? Right now, the documentation for the PWA template is mostly similar to the webpack template but there is some additional documentation which touches upon things like service worker.

Is there anything else that you'd like me to help with?

Edit: Tests are failing as well.

yarn test output
λ yarn test -w 1
yarn run v1.3.2
$ jest --env node -w 1
 FAIL  packages\@vue\cli-service-global\__tests__\globalService.spec.js (18.137s)
  ● global serve

    expect(received).toMatch(expected)

    Expected value to match:
      "Updated"
    Received:
      "hi"

      51 |       await nextUpdate() // wait for child stdout update signal
      52 |       await sleep(1000) // give the client time to update
    > 53 |       expect(await helpers.getText('h1')).toMatch(`Updated`)
      54 |     }
      55 |   )
      56 | })

      at serve (packages/@vue/cli-service-global/__tests__/globalService.spec.js:53:43)


 RUNS  packages/@vue/cli-service/__tests__/serve.spec.js
 ERROR  Error: EPERM: operation not permitted, mkdir 'E:\Projects\repos\vue-cli\packages\test\e2e-serve'
 RUNS  ERROR  Error: EPERM: operation not permitted, symlink 'E:\Projects\repos\vue-cli\packages\@vue\cli-service\bin\vue-cli-service' -> 'E:\Projects\repos\vue-cli\packages\test\e2e-serve-router\node_modules\.bin\vue-cli-service'
Error: EPERM: operation not permitted, symlink 'E:\Projects\repos\vue-cli\packages\@vue\cli-service\bin\vue-cli-service' -> 'E:\Projects\repos\vue-cli\packages\test\e2e-serve-router\node_modules\.bin\vue-cli-service'
    at Object.fs.symlinkSync (fs.js:1014:18)
    at setupDevProject (E:\Projects\repos\vue-cli\packages\@vue\cli\lib\util\setupDevProject.js:24:6)
    at Creator.create (E:\Projects\repos\vue-cli\packages\@vue\cli\lib\Creator.js:106:7)
    at <anonymous>
 FAIL  packages\@vue\cli-service\__tests__\serve.spec.js.js:188:7)
  ● serve

    Command failed: E:\Projects\repos\vue-cli\packages\@vue\cli\bin\vue create e2e-serve --force --config {"packageManager":"yarn","plugins":{"@vue/cli-plugin-babel":{},"@vue/cli-plugin-eslint":{"config":"base","lintOn":"save"},"@vue/cli-plugin-unit-mocha-webpack":{"assertionLibrary":"chai"}},"router":false,"vuex":false}

      at Promise.all.then.arr (node_modules/execa/index.js:236:11)

  ● serve with router

    Command failed: E:\Projects\repos\vue-cli\packages\@vue\cli\bin\vue create e2e-serve-router --force --config {"packageManager":"yarn","plugins":{"@vue/cli-plugin-babel":{},"@vue/cli-plugin-eslint":{"config":"base","lintOn":"save"},"@vue/cli-plugin-unit-mocha-webpack":{"assertionLibrary":"chai"}},"router":true,"vuex":false}

      at Promise.all.then.arr (node_modules/execa/index.js:236:11)


 RUNS  packages/@vue/cli-plugin-pwa/__tests__/pwa.spec.js
 ERROR  Error: EPERM: operation not permitted, symlink 'E:\Projects\repos\vue-cli\packages\@vue\cli-service\bin\vue-cli-service' -> 'E:\Projects\repos\vue-cli\packages\test\pwa-build\node_modules\.bin\vue-cli-service'
Error: EPERM: operation not permitted, symlink 'E:\Projects\repos\vue-cli\packages\@vue\cli-service\bin\vue-cli-service' -> 'E:\Projects\repos\vue-cli\packages\test\pwa-build\node_modules\.bin\vue-cli-service'
    at Object.fs.symlinkSync (fs.js:1014:18)
    at setupDevProject (E:\Projects\repos\vue-cli\packages\@vue\cli\lib\util\setupDevProject.js:24:6)
    at Creator.create (E:\Projects\repos\vue-cli\packages\@vue\cli\lib\Creator.js:106:7)
 FAIL  packages\@vue\cli-plugin-pwa\__tests__\pwa.spec.js
  ● pwa

    Command failed: E:\Projects\repos\vue-cli\packages\@vue\cli\bin\vue create pwa-build --force --config {"packageManager":"yarn","plugins":{"@vue/cli-plugin-babel":{},"@vue/cli-plugin-eslint":{"config":"base","lintOn":"save"},"@vue/cli-plugin-unit-mocha-webpack":{"assertionLibrary":"chai"},"@vue/cli-plugin-pwa":{}},"router":false,"vuex":false}

      at Promise.all.then.arr (node_modules/execa/index.js:236:11)


 RUNS  packages/@vue/cli-service/__tests__/build.spec.js
 FAIL  packages\@vue\cli-service\__tests__\build.spec.js\Projects\repos\vue-cli\packages\test\e2e-build'
  ● build

    Command failed: E:\Projects\repos\vue-cli\packages\@vue\cli\bin\vue create e2e-build --force --config {"packageManager":"yarn","plugins":{"@vue/cli-plugin-babel":{},"@vue/cli-plugin-eslint":{"config":"base","lintOn":"save"},"@vue/cli-plugin-unit-mocha-webpack":{"assertionLibrary":"chai"}},"router":false,"vuex":false}

      at Promise.all.then.arr (node_modules/execa/index.js:236:11)


 RUNS  packages/@vue/cli-plugin-unit-mocha-webpack/__tests__/mochaPlugin.spec.js
 ERROR  Error: EPERM: operation not permitted, symlink 'E:\Projects\repos\vue-cli\packages\@vue\cli-service\bin\vue-cli-service' -> 'E:\Projects\repos\vue-cli\packages\test\unit-mocha-webpack\node_modules\.bin\vue-cli-service'
Error: EPERM: operation not permitted, symlink 'E:\Projects\repos\vue-cli\packages\@vue\cli-service\bin\vue-cli-service' -> 'E:\Projects\repos\vue-cli\packages\test\unit-mocha-webpack\node_modules\.bin\vue-cli-service'
    at Object.fs.symlinkSync (fs.js:1014:18)
    at setupDevProject (E:\Projects\repos\vue-cli\packages\@vue\cli\lib\util\setupDevProject.js:24:6)
    at Creator.create (E:\Projects\repos\vue-cli\packages\@vue\cli\lib\Creator.js:106:7)
 FAIL  packages\@vue\cli-plugin-unit-mocha-webpack\__tests__\mochaPlugin.spec.js
  ● should work

    Command failed: E:\Projects\repos\vue-cli\packages\@vue\cli\bin\vue create unit-mocha-webpack --force --config {"packageManager":"yarn","useTaobaoRegistry":false,"plugins":{"@vue/cli-plugin-babel":{},"@vue/cli-plugin-unit-mocha-webpack":{"assertionLibrary":"chai"}}}

      at Promise.all.then.arr (node_modules/execa/index.js:236:11)


 RUNS  packages/@vue/cli-plugin-eslint/__tests__/eslintPlugin.spec.js
 ERROR  Error: EPERM: operation not permitted, symlink 'E:\Projects\repos\vue-cli\packages\@vue\cli-service\bin\vue-cli-service' -> 'E:\Projects\repos\vue-cli\packages\test\eslint\node_modules\.bin\vue-cli-service'
Error: EPERM: operation not permitted, symlink 'E:\Projects\repos\vue-cli\packages\@vue\cli-service\bin\vue-cli-service' -> 'E:\Projects\repos\vue-cli\packages\test\eslint\node_modules\.bin\vue-cli-service'
    at Object.fs.symlinkSync (fs.js:1014:18)
    at setupDevProject (E:\Projects\repos\vue-cli\packages\@vue\cli\lib\util\setupDevProject.js:24:6)
    at Creator.create (E:\Projects\repos\vue-cli\packages\@vue\cli\lib\Creator.js:106:7)
 FAIL  packages\@vue\cli-plugin-eslint\__tests__\eslintPlugin.spec.js
  ● should work

    Command failed: E:\Projects\repos\vue-cli\packages\@vue\cli\bin\vue create eslint --force --config {"packageManager":"yarn","useTaobaoRegistry":false,"plugins":{"@vue/cli-plugin-babel":{},"@vue/cli-plugin-eslint":{"config":"airbnb","lintOn":"commit"}}}

      at Promise.all.then.arr (node_modules/execa/index.js:236:11)

 PASS  packages\@vue\cli-service\__tests__\Service.spec.js
 PASS  packages\@vue\cli\__tests__\Generator.spec.js
 PASS  packages\@vue\cli\__tests__\Creator.spec.js
 PASS  packages\@vue\cli-plugin-eslint\__tests__\eslintGenerator.spec.js
 PASS  packages\@vue\cli\lib\promptModules\__tests__\unit.spec.js
 PASS  packages\@vue\cli\lib\promptModules\__tests__\eslint.spec.js
 PASS  packages\@vue\cli-plugin-unit-mocha-webpack\__tests__\mochaGenerator.spec.js
 PASS  packages\@vue\cli\lib\promptModules\__tests__\babel.spec.js
 PASS  packages\@vue\cli\__tests__\options.spec.js

Test Suites: 6 failed, 9 passed, 15 total
Tests:       7 failed, 46 passed, 53 total
Snapshots:   0 total
Time:        29.495s, estimated 49s
Ran all test suites.

@addyosmani
Copy link

I'm excited about this direction of encapsulating PWA functionality into a vue-cli plugin. I've handed maintainace of the PWA template to Jeff but am supportive of deprecating it in favor of the plugin once we can confirm feature parity.

Love the idea of no longer needing to keep in sync with the upstream changes to the webpack plugin. Thanks for looping us in, Evan. We'll take a look.

@addyosmani
Copy link

addyosmani commented Jan 10, 2018

Similar to the comment above, instructions on running the plugin for scaffolding would be greatly appreciated. Currently seems to error out.

@yyx990803
Copy link
Member Author

yyx990803 commented Jan 10, 2018

@sudo-suhas it seems to be Windows related.

  • What error do you see from yarn link?

  • The scaffolding error should be fixed in a300266, if it still has problems, try explicitly export VUE_CLI_DEBUG=true to your env before running vue create.

  • Tests are failing because fs.symlinkSync failed due to EPERM. Not sure why this is happening - tests are passing on Appveyor though.

@addyosmani if you are on macOS it should work as intended, let me know if you are running into problems as well.

@yyx990803
Copy link
Member Author

Re docs: we can have each plugin document itself in its README.md, and pull all the readmes together when building the docs into a single site.

@sudo-suhas
Copy link
Contributor

Yes, it is related to Windows(it usually is 😉).

What error do you see from yarn link?

I actually do not see any errors:

λ yarn link
yarn link v1.3.2
warning There's already a module called "@vue/cli" registered.
Done in 0.41s.

λ yarn unlink
yarn unlink v1.3.2
success Unregistered "@vue/cli".
info You can now run `yarn unlink "@vue/cli"` in the projects where you no longer want to use this module.
Done in 0.35s.

λ yarn link
yarn link v1.3.2
success Registered "@vue/cli".
info You can now run `yarn link "@vue/cli"` in the projects where you want to use this module and it will be used instead.
Done in 0.33s.

I got an error when I tried this the first time because I had the stable version of vue-cli installed globally. But I have since removed that and as you can see above, I have tried unlink and link but that does not help.

The scaffolding error should be fixed in dc84b7e

Still faced the previous issue.

if it still has problems, try explicitly export VUE_CLI_DEBUG=true to your env before running vue create.

Tried this, but it fails due to symlinks.

Vue CLI v3.0.0-alpha.1 DEBUG
✨  Creating project in E:\Projects\repos\vue-cli\packages\test\test-app.
🗃  Initializing git repository...
 ERROR  Error: EPERM: operation not permitted, symlink 'E:\Projects\repos\vue-cli\packages\@vue\cli-service\bin\vue-cli-service' -> 'E:\Projects\repos\vue-cli\packages\test\test-app\node_modules\.bin\vue-cli-service'
Error: EPERM: operation not permitted, symlink 'E:\Projects\repos\vue-cli\packages\@vue\cli-service\bin\vue-cli-service' -> 'E:\Projects\repos\vue-cli\packages\test\test-app\node_modules\.bin\vue-cli-service'
    at Object.fs.symlinkSync (fs.js:1014:18)
    at setupDevProject (E:\Projects\repos\vue-cli\packages\@vue\cli\lib\util\setupDevProject.js:24:6)
    at Creator.create (E:\Projects\repos\vue-cli\packages\@vue\cli\lib\Creator.js:106:7)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)

Regarding symlinks, I have a couple of questions

  • This is only required for development right? If so, I saw some node issues(nodejs/node-v0.x-archive issues 2274, 6342) which suggest that running the terminal with admin privileges should it.
  • Can lerna bootstrap help with this?
  • Is there any way we can do this without using symlinks? I'd prefer to not use admin privileges if possible. In my experience, this tends to cause unexpected problems.

@yyx990803
Copy link
Member Author

@sudo-suhas so turns out you need to specify type to be 'junction' when using fs.symlink on windows. Figured this out by reading npm/yarn's source code... Let me know if 071beec fixes it for you.

@jeffposnick
Copy link
Contributor

That sounds like a solid plan, @yyx990803. And thanks for continuing to work on this, @sudo-suhas—I'm happy to follow your lead if you're able to get the initial port working. Do you have your experimental code pushed anywhere public right now?

One of the improvements that I'm interested in making with the PWA template is switching over from SWPrecacheWebplackPlugin to the workbox-webpack-pliugin library. And then there are some preload-webpack-plugin improvements, starting with GoogleChromeLabs/preload-webpack-plugin#51, to make as well.

@yyx990803
Copy link
Member Author

@jeffposnick switching the plugin should be dead simple, just swap out this part as long as it generates the same /service-worker.js file.

As for preload plugin - I have it included in the base setup by default now with a forked version here. When my change for include: 'initial'is merged or you guys publish 3.0 I can switch over.

@sudo-suhas
Copy link
Contributor

@yyx990803 I managed to advance a few steps after the recent bug fixes you made. However, the scaffolding now fails in the lint step. Here's the debug logs - vue-create-app.debug.txt

I tried running the lint script in the test-app but that too failed:

λ yarn lint
yarn run v1.3.2
$ vue-cli-service lint
'vue-cli-service' is not recognized as an internal or external command,
operable program or batch file.

thanks for continuing to work on this, @sudo-suhas

It's pretty awesome that I get to collaborate with the people I admire and look up to. So it's a win-win 😄.

Do you have your experimental code pushed anywhere public right now?

Right now, the packages the test-app depends on aren't published and they are instead supposed to work via symlinks to binaries in the monorepo. I have published the app which I just created here - sudo-suhas/vue-pwa-next. But this will mostly be just a read-only repo.

@yyx990803
Copy link
Member Author

@sudo-suhas that error log is again quite weird... it seems you don't have register-service-worker and @vue/test-utils installed locally, possibly due to an outdated lockfile... try run yarn clean and then yarn again and make sure both of these deps are found in yarn.lock.

@sudo-suhas
Copy link
Contributor

try run yarn clean and then yarn again

My apologies, was able to create the app without errors after this. But the symlink isn't working correctly:

λ yarn serve
yarn run v1.3.2
$ vue-cli-service serve
'vue-cli-service' is not recognized as an internal or external command,
operable program or batch file.
error Command failed with exit code 1.

I'll try to look into this since it will be hard to debug without a Windows machine.

@sudo-suhas
Copy link
Contributor

So here's my findings for symlinks:

Unfortunately, even fs.linkSync won't work:

# First, removed the junction synlink manually
λ node
> fs.linkSync('E:\\Projects\\repos\\vue-cli\\packages\\@vue\\cli-service\\bin\\vue-cli-service', 'E:\\Projects\\repos\\vue-cli\\packages\\test\\vue-pwa-next\\node_modules\\.bin\\vue-cli-service')

> .exit

# File got linked properly
λ head -5 .\node_modules\.bin\vue-cli-service
#!/usr/bin/env node

const semver = require('semver')
const { error } = require('@vue/cli-shared-utils')
const requiredVersion = require('../package.json').engines.node

# `#!/usr/bin/env node` doesn't work on Windows, need the `vue-cli-service.cmd` that npm/yarn creates
λ yarn serve
yarn run v1.3.2
$ vue-cli-service serve
'vue-cli-service' is not recognized as an internal or external command,
operable program or batch file.
error Command failed with exit code 1.

The only way that I can think of solving this is using cmd-shim. Both npm and yarn use this to workaround symlink limitations on Windows. Shall I do it?

@yyx990803
Copy link
Member Author

@sudo-suhas ha, I didn't realize it's such a hassle to symlink an executable in windows... yeah I guess we can just do what yarn is doing here

@sudo-suhas
Copy link
Contributor

Okay, I'll do these changes and make a PR to the next branch latest by tomorrow.

@igogrek igogrek mentioned this issue Jan 11, 2018
3 tasks
@yyx990803
Copy link
Member Author

@sudo-suhas I got that in db9be90 so I can have others using windows test it, but thanks for investigating this!

@sudo-suhas
Copy link
Contributor

yarn serve worked correctly after I pulled in the latest changes.

As a side note, would be willing to accept a PR which switches over to cosmiconfig to load the config?

@yyx990803
Copy link
Member Author

@sudo-suhas I intentionally didn't use it because we only want the config file in one deterministic location, plus, .vuerc (global creation preference) and vue.config.js (per-project config) have different meanings.

@jeffposnick
Copy link
Contributor

I just spent some time looking through the new plugin architecture in more detail—it's really nice, and I was happy to see that so much of cli-plugin-pwa is already working!

I have a branch that uses the current alpha release of Workbox instead of sw-precache at https://github.com/vuejs/vue-cli/compare/next...jeffposnick:workbox-pwa?expand=1

One thing that I'm not sure about if whether there's anything I could do to expose the options passed to the Workbox webpack plugin to the developer. I could imagine, for instance, that a developer might want to pass in an additional runtimeCaching property when configuring the Workbox plugin. This is the sort of thing that, with create-react-app, you'd have to eject in order to configure, but my impression from reading through #589 (comment) and elsewhere is that the config chaining means that ejecting isn't needed. So how would a developer end up making a slight tweak to those config options?

@yyx990803, are you interested in a PR for that change, given that it's still an alpha release of Workbox? In other words, are you okay with merging in code to the next branch that will have to change before the final vue-cli release?

@yyx990803
Copy link
Member Author

@jeffposnick good to know!

The second options argument the plugin receives is the user's own vue.config.js or "vue" field in package.json, you can read fields on there to allow users to customize the behavior of your plugin. For example, if the user has the following vue.config.js:

module.exports = {
  pwa: {
    workBoxPluginOptions: { ... }
  }
}

Then in the plugin you can read:

module.exports = (api, options) => {
  const userOptions = options.pwa || {}
  const workBoxOptions = Object.assign(defaults, userOptions.workBoxOptions)
}

As for workbox, how far along in the future do you think it will stabilize? We still have some work left before we officially release vue-cli 3.0, so it would be ideal to coordinate. If we don't have something stable by the time we are ready to release then I think it's better to stick with sw-precache for the moment.

@jeffposnick
Copy link
Contributor

Thanks, @yyx990803. The userOptions.workBoxOptions pattern sounds like it will play nicely with what we need to provide Workbox.

We're shooting for a Workbox v3 final release by the end of January. We're at the stage where we're just testing out various integrations (like this one) and fixing any bugs/pain points that remain. How does that track with the vue-cli v3 timeline?

@yyx990803
Copy link
Member Author

yyx990803 commented Jan 16, 2018

@jeffposnick that should work out nicely! We'll most likely still be in alpha/beta state around that time. We can take a PR to switch to workbox once v3 is released now and move to v3 stable once that's released.

@yyx990803
Copy link
Member Author

Update: we have started making alpha releases! You can now directly test the new CLI by following the guide in README.

@yyx990803
Copy link
Member Author

@jeffposnick any updates regarding workbox v3? Open to a PR to migrate to workbox-webpack-plugin with sane defaults.

@jeffposnick
Copy link
Contributor

We just hit beta yesterday (rather than final, as was our original goal): https://github.com/GoogleChrome/workbox/releases/tag/v3.0.0-beta.0

I'm happy to file a PR to migrate to the beta release to coincide with the alpha releases of the CLI. Stay tuned.

@nanomad
Copy link
Contributor

nanomad commented Feb 13, 2018

For what is worth, I've just migrated a personal POC app that used the PWA template along with vuex and the migration when smoothly.

I'll test it in the next days to see if anything comes up, but for now the experience has been painless

@multiplegeorges
Copy link

I moved an existing project over to using this template with the cli beta and I found that the service worker didn't behave as I expected with the existing configuration.

Problem:

The current configuration only returns index.html from the cache when the root of the app is called directly, as opposed to a link into the app like app.dom/login/new, for example.

This leads to "No Internet Connection" errors in the browser because nothing in the cache matches the route requested.

I expected navigation calls to always return index.html from the cache in order to support History mode routing in vue-router when the app is offline.

Proposal:

Change the workbox default settings in vue.config.js to:

workboxPluginMode: 'InjectManifest',
workboxOptions: {
  // swSrc is required in InjectManifest mode.
  swSrc: 'src/service-worker.js'
}

And the default contents of src/service-worker.js to be:

/* globals workbox */
// Workbox is automatically imported when this is built for production.
workbox.precaching.precacheAndRoute(self.__precacheManifest)
workbox.routing.registerNavigationRoute('/index.html')

// Service worker custom application code goes below this line.

I think this has two main benefits:

  • Developers will expect that routing just works, even with SW caching, and this accomplishes that.
  • It gives developers a location in the project to begin putting their custom SW code, such as support for push notifications.

If this sounds good, I'm happy to put together a PR with these changes.

@jeffposnick
Copy link
Contributor

To give some perspective, create-react-app shipped with a similar integration, with navigation fallbacks to index.html enabled by default. While it met some developers' expectations, it also caused enough consternation that it ended up being disabled: facebook/create-react-app#3248

vue-cli's templates, especially in v3, is wonderful in that it gives developers more flexibility to adjust the configuration used for service worker generation, so in some senses the default behavior doesn't matter as much. As long as there's a well-documented way to enable to disable those behaviors, I think developers will have enough control to do what makes sense for them.

My gut was to ship with navigation fallbacks disabled by default, and then we can explain to developers that want that behavior to make the following changes to their configuration in order to enable it. (You don't actually have to go the InjectManifest route, since there's a simple config option that could be used with GenerateSW.)

workboxOptions: {
  navigateFallback: 'index.html',
}

If there's consensus, I'm not against making that configuration (along with GenerateSW mode) the default, along with documentation that disabling that behavior requires a configuration like:

workboxOptions: {
  navigateFallback: undefined,
}

@multiplegeorges
Copy link

Thanks for the background on this issue from create-react-app. If I understand that issue correctly, it stemmed from the requirement that you eject in order to gain the extra functionality that wasn't supported out of the box. Since we can re-invoke using the CLI, I think that issue doesn't apply to this project. Maybe that means this should all just be an option during the invoke steps and let devs choose their path?

But, if not...

It's my view that supporting pushState navigation out of the box is a very desirable feature, especially in a PWA template that will almost always have a single app shell html file.

Some background on how I came into this: I generated a project from this template and I was looking for somewhere to hook in some push notification subscription/handlers. As I had never done this before, let alone integrated it with this template or the Vue build process, it wasn't clear to me where it should go.

I see two ways this could be improved:

Config Options

With these options:

{
  workboxPluginMode: 'GenerateSW',
  workboxOptions: {
    navigateFallback: 'index.html',
    importScripts: ['/custom-service-worker.js']
  }
}

Benefits:

  • Minimal changes to the existing way of doing things
  • Workbox generates everything

Downsides:

  • custom-service-worker.js needs to be in approot/public so that it can be imported properly
  • custom-service-worker.js doesn't get processed during build

InjectManifest

With these options:

{
  workboxPluginMode: 'InjectManifest',
  workboxOptions: {
    swSrc: 'src/service-worker.js'
  }
}

And the generated/default content of service-worker.js being the same as my initial message.

Benefits:

  • Avoids more files in public, keeps app code in src
  • Clear entry point for developers to start using service worker features: put service worker code in... service-worker.js

Downsides:

  • Higher mental load, it forces developers to deal with Workbox calls when that could be completely hidden
  • If a developer doesn't heed the warning to not modify the beginning of service-worker.js, they'll potentially lose functionality for reasons that may not be clear to them. This is much less likely to happen with config options.

I'm for either of these solutions, with a slight preference for the second one.

In any case, this template is really great and I went from an existing traditional SPA to a PWA with 90+ Lighthouse score in a couple of hours. 👍

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

No branches or pull requests

6 participants