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

Jump to bottom

Fix Tapable.plugin warning and supports assetPrefix in register-sw #78

Merged
merged 4 commits into from
Nov 16, 2018
Merged

Fix Tapable.plugin warning and supports assetPrefix in register-sw #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6b5c070
Remove warning: Tapable.plugin is deprecated. by using hooks
mirgj Nov 14, 2018
12a4836
Add in support for assetPrefix in register-sw.js to be able to regist…
mirgj Nov 14, 2018
6a7d576
Replace fs with fs-extra for consistency
mirgj Nov 14, 2018
75cf241
Refactoring
Nov 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ node_modules/
tests/.next
*.log
tests/node_modules
tests/static/manifest/
tests/static/manifest/
register-sw-compiled.js
9 changes: 8 additions & 1 deletion index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const { GenerateSW, InjectManifest } = require('workbox-webpack-plugin');
const CleanWebpackPlugin = require('clean-webpack-plugin');
const CopyWebpackPlugin = require('copy-webpack-plugin');
const { join } = require('path');
const { readFile, writeFile } = require('fs-extra');

const InlineNextPrecacheManifestPlugin = require('./plugin');
const exportSw = require('./export');
Expand All @@ -21,6 +22,7 @@ module.exports = (nextConfig = {}) => ({
generateSw = true,
dontAutoRegisterSw = false,
devSwSrc = join(__dirname, 'service-worker.js'),
registerSwPrefix = assetPrefix || '',
workboxOpts = {
globPatterns: ['static/**/*'],
globDirectory: '.',
Expand Down Expand Up @@ -50,7 +52,12 @@ module.exports = (nextConfig = {}) => ({
config.entry = async () => {
const entries = await originalEntry();
if (entries['main.js'] && !dontAutoRegisterSw) {
entries['main.js'].unshift(require.resolve('./register-sw.js'));
let content = await readFile(require.resolve('./register-sw.js'), 'utf8');
content = content.replace('{REGISTER_SW_PREFIX}', registerSwPrefix);

await writeFile(join(__dirname, 'register-sw-compiled.js'), content, 'utf8');
Copy link
Owner

Choose a reason for hiding this comment

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

not certain about this block because service workers must be served on the origin domain they're being used on.

So, the service worker on jackhanford.com would never register if it was being served on cdn.foobar.com.

w3c/ServiceWorker#940

Copy link
Owner

Choose a reason for hiding this comment

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

ahh, disregard ^ if this is for serving the service worker on a different path on the same domain. ie jackhanford.com/foo/bar/service-worker.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.

@hanford yes it's actually to load it from a different path in case your application resides on a virtual directory

eg. my application is on test.com/food/bar but its trying to get the service worker from test.com/; this because the register script fetch the service-worker from the root folder.


entries['main.js'].unshift(require.resolve('./register-sw-compiled.js'));
}
return entries;
};
Expand Down
26 changes: 17 additions & 9 deletions plugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,22 @@ module.exports = class InlineNextPrecacheManifestPlugin {
}

apply(compiler) {
compiler.plugin(
'done',
async () => {
await generateNextManifest(this.opts);
},
err => {
throw new Error(`Precached failed: ${err.toString()}`);
},
);
const errorhandler = err => {
throw new Error(`Precached failed: ${err.toString()}`);
};

if (compiler.hooks) {
compiler.hooks.done.tap(
'CopyPlugin',
async() => generateNextManifest(this.opts),
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 should be ok now, tested and seems working well

errorhandler
);
} else {
compiler.plugin(
'done',
async() => generateNextManifest(this.opts),
errorhandler
);
}
}
};
2 changes: 1 addition & 1 deletion register-sw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
if ('serviceWorker' in navigator) {
window.addEventListener('load', function() {
navigator.serviceWorker
.register('/service-worker.js')
.register('{REGISTER_SW_PREFIX}/service-worker.js')
.then(function(registration) {
console.log('SW registered: ', registration);
})
Expand Down