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.

Sign up for GitHub

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
Closed

Import scripts in Service Worker #2714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/react-scripts/config/paths.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?

: undefined;

// config after eject: we're in ./config/
module.exports = {
dotenv: resolveApp('.env'),
Expand All @@ -62,6 +66,7 @@ module.exports = {
appNodeModules: resolveApp('node_modules'),
publicUrl: getPublicUrl(resolveApp('package.json')),
servedPath: getServedPath(resolveApp('package.json')),
sWPrecacheImportScript: sWPrecacheImportScript,
};

// @remove-on-eject-begin
Expand All @@ -82,6 +87,7 @@ module.exports = {
appNodeModules: resolveApp('node_modules'),
publicUrl: getPublicUrl(resolveApp('package.json')),
servedPath: getServedPath(resolveApp('package.json')),
sWPrecacheImportScript: sWPrecacheImportScript,
// These properties only exist before ejecting:
ownPath: resolveOwn('.'),
ownNodeModules: resolveOwn('node_modules'), // This is empty on npm 3
Expand Down
1 change: 1 addition & 0 deletions packages/react-scripts/config/webpack.config.prod.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ module.exports = {
// about it being stale, and the cache-busting can be skipped.
dontCacheBustUrlsMatching: /\.\w{8}\./,
filename: 'service-worker.js',
importScripts: paths.sWPrecacheImportScript ? [paths.sWPrecacheImportScript] : undefined,
logger(message) {
if (message.indexOf('Total precache size is') === 0) {
// This message occurs for every build and is a bit too noisy.
Expand Down