[v5-alpha] workbox-build does not bundle workbox-* packages with rollup

[kaykayehnn]

Library Affected:
workbox-build

Browser & Platform:
all browsers, tested on macOS and Windows

Issue or Feature Request Description:
In v5 alpha, when workbox-build generates a service worker, it saves it to a temporary directory on the file system in order to pass it to rollup for bundling. The temporary directory only contains sw.js and rollup cannot resolve the imports to workbox-* packages, so it assumes they are external dependencies and leaves them as raw define calls.

Expected behaviour
Workbox-build generates a bundle containing the service worker and its dependencies.

Actual behaviour
Workbox-build generates a bundle containing raw define calls to dependent packages.

Minimal reproducible repo
kaykayehnn/workbox-build-node-resolution-bug

Notes
rollup-plugin-node-resolve, which is used to resolve imports in the service worker bundle, has an option to provide a base directory for resolutions. Setting it to the project root should fix this issue.
I can make a PR for this if that's okay.

[jeffposnick]

Thanks for reporting this. I'm taking a look now. The intended behavior is for workbox-build to figure out an absolute path to the various dependencies, but I guess that's not working as expected:

workbox/packages/workbox-build/src/lib/module-registry.js

Lines 22 to 23 in 9a4b382

	this.nodeModulesPath = upath.resolve(
	__dirname, '..', '..', 'node_modules');

[jeffposnick]

Okay, I think I see what's happening: when you use workbox-build "in the real world", then the other Workbox libraries don't end up installed within a node_modules subdirectory of the workbox-build project like they do locally in the Lerna-managed repo.

I'll clean up that logic. Thanks for reporting it!

[kaykayehnn]

Sure! I put some notes I took while debugging the issue in a gist in case they might be helpful. You can take a look at the revisions for a diff of the changes.

[jeffposnick]

Thanks for the debugging and proposed solution, @kaykayehnn! I tested out your change and it looks good:

getImportStatements() {
  const workboxModuleImports = [];

  for (const [localName, {moduleName, pkg}] of this.modulesUsed) {
    const pkgJsonPath = require.resolve(`${pkg}/package.json`);
    const pkgRoot = upath.dirname(pkgJsonPath);
    const importStatement = ol`import {${moduleName} as ${localName}} from
      '${pkgRoot}/${moduleName}.mjs';`;

    workboxModuleImports.push(importStatement);
  }

  return workboxModuleImports;
}

The one slightly messy part is that there's a unit test that needs to be updated to work with this new approach. We use proxyquire to mock out various things when running our unit tests, but you can't use it on require.resolve() (which kind of makes sense), so the test case ends up looking a little messy.

If you wanted to open a PR against the v5 branch that includes your modifications to getImportStatements(), I'm happy to force-push an update to the test suite so that you didn't have to worry about it.

[kaykayehnn]

Fixed in #2110