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

Fixes workbox dependencies #1667

Merged
merged 2 commits into from Sep 28, 2018

Conversation

arcanis
Copy link
Contributor

@arcanis arcanis commented Sep 28, 2018

cc @jeffposnick @philipwalton (didn't open an issue since both the problem and fix are relatively straightforwards)

Description of what's changed/fixed.

The workbox-webpack-plugin is trying to access the babel-runtime package (because it gets transformed using babel-plugin-transform-runtime), which isn't declared in its dependencies. As a result package managers offer no guarantee as to which version will be provided, or even if babel-runtime will be installed at all.

This diff simply adds the dependency that's missing, based on what I found in the package-lock.json at the root of the repository.

@jeffposnick
Copy link
Contributor

Thanks—this is something that I caught when doing a dependency update sweep in preparation for our v4 release: https://github.com/GoogleChrome/workbox/pull/1658/files#diff-d9674ca55441128e3145e9552ce61a71R31

I believe that we were getting by because workbox-build was pulling in the babel-runtime package implicitly, but it's obviously preferable to have it explicitly listed, as it should be in that PR.

My personal preference would be to just defer to that upcoming change in v4 unless this is something that is actively causing a failure for you with the current v3 release.

@arcanis
Copy link
Contributor Author

arcanis commented Sep 28, 2018

I noticed it while working on facebook/create-react-app#5136, which adds support for Plug'n'Play to create-react-app. Since package boundaries are enforced under this environment, it throws an error:

screen shot 2018-09-28 at 3 58 15 pm

@jeffposnick
Copy link
Contributor

Gotcha—Okay, we can merge this in for a 3.6.2 release given that.

@jeffposnick
Copy link
Contributor

Thanks—I was just going to ask about the depcheck 😄

@coveralls
Copy link

Coverage Status

Coverage remained the same at 86.079% when pulling debb46f on arcanis:fixes-workbox-dependencies into 3612461 on GoogleChrome:master.

@jeffposnick jeffposnick merged commit 9666462 into GoogleChrome:master Sep 28, 2018
@arcanis
Copy link
Contributor Author

arcanis commented Sep 28, 2018

Perfect, thanks!

@jeffposnick
Copy link
Contributor

3.6.2 is out with the fix: https://github.com/GoogleChrome/workbox/releases/tag/v3.6.2

Thanks again!

jeffposnick added a commit that referenced this pull request Oct 3, 2018
* WIP.

* WIP.

* Linting.

* Removing browser-sync.

* Fixes workbox dependencies (#1667)

* Fixes workbox dependencies

* Fixes the depcheck

* v3.6.2

* 3.6.2 metadata (#1669)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants