Skip to content

Conversation

@lukasdotcom
Copy link
Contributor

@lukasdotcom lukasdotcom commented Jul 28, 2022

In release 5.5.5 the disable option was broken. This fixes the option and allows it to work again.
Edit: Turns out all of the options specified do not work in 5.5.5. This should fix this.
Resolves: #371
Resolves: #370
Resolves: #378

Copy link

@theoludwig theoludwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍
Thanks, it solves the issue.

Just tested by manually editing my node_modules. 😄

@sanderkooger
Copy link

Just subscribing so i know when this is merged. Having th same issue

@MaheshtheDev
Copy link

Just subscribing to know when this will merge, because having same issue

@lukasdotcom lukasdotcom changed the title Fixes disable option Allows all PWA options to work Aug 3, 2022
Copy link

@digitaluprising digitaluprising left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@PicchiKevin PicchiKevin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@brenopolanski
Copy link

brenopolanski commented Aug 5, 2022

Just tested by manually editing my node_modules.

I did the same test and it worked 😄

i'm using [email protected] & next-pwa@^5.5.5

@vdjurdjevic
Copy link

Just subscribing to know when this will merge

@theoludwig
Copy link

cc @shadowwalker

@bdlb77
Copy link

bdlb77 commented Aug 8, 2022

+1 to changes in source code working via manual change to my node_modules

Copy link

@d-strygwyr d-strygwyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@d3ni00
Copy link

d3ni00 commented Aug 9, 2022

For me, #381 is more the next.js way to fix it.

@Diesmo
Copy link

Diesmo commented Aug 9, 2022

For me, #381 is more the next.js way to fix it.

I agree with this.
This PR here is merely a hotfix/workaround for the breaking change introduced in v5.5.5 because of the next.js changes.

#381 on the other hand is dealing with the config how next.js intented it (at least thats how I understood the next.js changes that lead to the situation we are in now)

I suggest to merge #381 instead of this PR here.

@brenopolanski
Copy link

For me, #381 is more the next.js way to fix it.

Yeah! Looking at the changes in PR #381 I also agree that it's the best way to fix the issue.

@lekipising
Copy link

Having same issue, subscribing

@kirkegaard
Copy link

LGTM

@shadowwalker
Copy link
Owner

OK, I have read through the discussions and code changes in these PRs:

I will take this PR as it's minimal change and doesn't include format changes, will release a patch version 5.5.6.

Beside will make a new breaking change and release version 5.6.0 soon. Following the guidance from official plugin implementation.

@shadowwalker shadowwalker merged commit c3d6836 into shadowwalker:master Aug 21, 2022
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.

service worker script returns 404 'disable' option not disabling pwa feature in development mode. Service Worker not generated in public folder