-
-
Notifications
You must be signed in to change notification settings - Fork 375
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
fix: Correcting SW flag in dev #1534
Conversation
🦋 Changeset detectedLatest commit: fae2269 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@@ -103,6 +117,38 @@ async function clientConfig(env) { | |||
}, | |||
].filter(Boolean) | |||
), | |||
...(env.sw |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this makes things a lot less readable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nested ternary?
Yeah, sorry, can't remember why I did it like that. I'll try to fix that up tomorrow (3am here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed an update, hopefully that's more readable.
fdff16d
to
b3625ea
Compare
What kind of change does this PR introduce?
Bug fix
Did you add tests for your changes?
No
Summary
Running
yarn dev --sw
would create an error thatprocess
was not defined. This is because there was noDefinePlugin
entry forprocess.env.ES_BUILD
on dev. This was found here #1531 (comment)Upon adding that, the service worker would attempt to be installed but run into errors upon registering. The solution for that seems to be adding the
InjectManifest
as a plugin, though this isn't ideal and throws a workbox warning (GoogleChrome/workbox#1790). Kinda seems like we just shouldn't support the non-debug SW during dev?Additionally, this still fails if ran with the
--esm
flag, as nosw-esm.js
is generated in dev. But that's probably a separate issue.Does this PR introduce a breaking change?
No