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

Import scripts in Service Worker #2714

Closed
wants to merge 1 commit into from

Conversation

piotr-cz
Copy link
Contributor

@piotr-cz piotr-cz commented Jul 3, 2017

This PR adds an ability to use importScripts option of SWPrecacheWebpackPlugin.

How-to:

  1. create a file called public/service-worker-import.js
  2. run npm run build
  3. check that file has been loaded (either in console network panel or by placing console.log statement inside and checking for output) by refreshing the page

File is being imported only on service worker install/ update.

Downsides:

  • It's possible to import only one file and it's name is harcoded
  • File is not being passed trough plugins (minification, etc)

Fixes: #2253

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@@ -48,6 +48,10 @@ function getServedPath(appPackageJson) {
return ensureSlash(servedUrl, true);
}

const sWPrecacheImportScript = fs.existsSync(resolveApp('public/service-worker-import.js'))
? 'service-worker-import.js'
Copy link
Contributor

Choose a reason for hiding this comment

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

We generally use camelCase convention for filenames.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an abbreviation and as such I thought sWPrecacheImportScript should be fine. Are you suggesting sWprecacheImportScript ?

Copy link
Contributor

Choose a reason for hiding this comment

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

He's suggesting serviceWorkerImport.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Build script is creating a service-worker.js file - that's why I decided to use service-worker-import.js.

I guess there is such naming convention for files which are served from public directory.

Unlike the testsSetup.js which is located in src dir, this one should be placed in public and is just copied over as it is into build by build script.

Copy link

Choose a reason for hiding this comment

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

Is this a blocking issue/preventing further review?

@piotr-cz
Copy link
Contributor Author

piotr-cz commented Jul 4, 2017

I'd prefer if the import script would be located in the src directory and build script would run all the manipulations on it (search & replace of variables and minification) but I don't know how to - that's why I'm using this package.

@uniquename
Copy link

What's the reason that this is not getting implemented? Would be convenient to be able to customize service worker without ejecting.

@chemitaxis
Copy link

Hi! Any news about it? Thanks!

@scrombie
Copy link

scrombie commented Oct 9, 2017

Hi all! Please, any info on this?

@react-scripts-dangerous
Copy link

Hello! I'm a bot that helps facilitate testing pull requests.

Your pull request (commit c566e73) has been released on npm for testing purposes.

npm i [email protected]
# or
yarn add [email protected]
# or
create-react-app [email protected] folder/

Note that the package has not been reviewed or vetted by the maintainers. Only install it at your own risk!

Thanks for your contribution!

@uniquename
Copy link

uniquename commented Oct 18, 2017

When I run it with [email protected] and then yarn start, I get this error

./~/react-error-overlay-dangerous/lib/components/code.js Module not found: Can't resolve 'react-dev-utils/ansiHTML' in '/Users/hendrik/werkbank/ba-react/folder/node_modules/react-error-overlay-dangerous/lib/components'

@Timer
Copy link
Contributor

Timer commented Oct 18, 2017

Hmm, it seems react-dev-utils/ansiHTML isn't being replaced with react-dev-utils-dangerous/ansiHTML, most peculiar!

Sorry @uniquename, but this released cut won't work -- I can look into fixing this for future cuts.

@Paduado
Copy link

Paduado commented Nov 17, 2017

Hi! Is there any chance this would get implemented? Or should I just eject

@tszarzynski
Copy link

We had a similar problem while working on a project recently and we didn't want to "eject". Instead, we created a little tool that allows you to append custom service worker code to the one generated by CRA. It's not an ideal solution but worked well for us.

Have a look here: https://github.com/bbhlondon/cra-append-sw

@Eder87rh
Copy link

Any news?, i dont want to eject

@marcelkornblum
Copy link

@Eder87rh try out our tool that @tszarzynski mentioned above (https://github.com/bbhlondon/cra-append-sw) - this PR has been open almost 4 months now and it's an easy way to avoid ejecting if that's your only reason to do it.

@jamland
Copy link

jamland commented Nov 27, 2017

Would be cool to have this feature!

btw https://github.com/bbhlondon/cra-append-sw rains with errors for me

@uniquename
Copy link

It might be possible, that there is not much going on here, because workbox will replace swprecache sooner or later #2340 (comment)

@marcelkornblum
Copy link

@jamland could you post an issue please? Sorry it didn't work for you, we built it as we were using it on a project ourselves

@jamland
Copy link

jamland commented Nov 28, 2017

@marcelkornblum sure I will!
Yes, I understand / thanks for your effort anyway =)

@DanPen
Copy link

DanPen commented Dec 8, 2017

Can we get this merged? I'm in dire need :)

@anshul
Copy link

anshul commented Dec 27, 2017

Any updates?

@piotr-cz
Copy link
Contributor Author

piotr-cz commented Jan 17, 2018

I have no idea why this is still pending.

@Oupsla
Copy link

Oupsla commented Apr 4, 2018

Little ping to @gaearon and @Timer
Can we have an update on this PR ? Thanks !

@xuzhanhh
Copy link

xuzhanhh commented Apr 6, 2018

can we custom sw code in cra without ejecting now?

@Timer
Copy link
Contributor

Timer commented Sep 26, 2018

Closing in favor of #4169 which will make the service worker fully configurable.

@Timer Timer closed this Sep 26, 2018
@lock lock bot locked and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.