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

Return back service worker which generated by react-scripts #1426

Closed
wants to merge 1 commit into from

Conversation

Luchanso
Copy link
Contributor

Now service worker make unnecessary optimisation for API fetches - it's not good, because we make duplicate requests.
image

Service worker from react-scripts very good configure.

@howardchung
Copy link
Member

@superoo7 implemented the custom service worker in #1344

We had an issue with the default CRA service worker not working after some updates.

Would we have to unregister the existing service worker before deploying this to avoid conflicts?

@Luchanso
Copy link
Contributor Author

Luchanso commented Jan 19, 2018

Yes, of course, we need disable old service worker. I will disable it in this PR.

@superoo7
Copy link
Contributor

I am quite busy recently, but will try to find some time to check it out.

Some general service worker specification.

  • Service Worker's file name should not never be changed.
  • If name does not change, when service worker change any slightest of code, it will update automatically.

What I did in my previous PR:

  • I write the service worker code manually without any tools like workbox
  • I created register event, and activate event.

@superoo7
Copy link
Contributor

some side notes about service worker generated by react-script, it used SWPrecacheWebpackPlugin to generate service worker file, which is the stable and older tools to generate service worker.

You can read through what is written in previous suggestion

@howardchung
Copy link
Member

I would still like to use the CRA service worker if we can figure out what the issue is (convention over configuration).

The issue seems to stem from the JS bundle name changing between updates, but this is something that create-react-app already does (putting the hash into the file name to prevent stale fetches), so I'm surprised that it's not working. Maybe something has changed in the last few months to make it more consistent?

@superoo7
Copy link
Contributor

I will make a PR, to make a dynamic fetching using the fetch event listener in the service worker.

Maybe I will do it over the weekend.

@Luchanso
Copy link
Contributor Author

I think this PR is not so good idea, because Dan says service worker is have some pitfalls https://twitter.com/dan_abramov/status/954146978564395008 and have some issue facebook/create-react-app#2554

@Luchanso Luchanso closed this Jan 22, 2018
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