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

Service Worker HTML caching bug fix #2390

Merged
merged 10 commits into from
May 14, 2020
Merged
19 changes: 2 additions & 17 deletions packages/peregrine/lib/talons/App/useApp.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useEffect, useMemo, useState } from 'react';
import { useCallback, useEffect, useMemo } from 'react';
import errorRecord from '@magento/peregrine/lib/util/createErrorRecord';
import { useAppContext } from '@magento/peregrine/lib/context/app';

Expand Down Expand Up @@ -33,19 +33,11 @@ export const useApp = props => {
handleError,
handleIsOffline,
handleIsOnline,
handleHTMLUpdate,
markErrorHandled,
renderError,
unhandledErrors
} = props;

const [isHTMLUpdateAvailable, setHTMLUpdateAvailable] = useState(false);

const resetHTMLUpdateAvaialable = useCallback(
() => setHTMLUpdateAvailable(false),
[setHTMLUpdateAvailable]
);

const reload = useCallback(
process.env.NODE_ENV === 'development'
? () => {
Expand Down Expand Up @@ -98,19 +90,12 @@ export const useApp = props => {
}
}, [handleIsOnline, handleIsOffline, hasBeenOffline, isOnline]);

useEffect(() => {
if (isHTMLUpdateAvailable) {
handleHTMLUpdate(resetHTMLUpdateAvaialable);
}
}, [isHTMLUpdateAvailable, handleHTMLUpdate, resetHTMLUpdateAvaialable]);

const handleCloseDrawer = useCallback(() => {
closeDrawer();
}, [closeDrawer]);

return {
hasOverlay: !!overlay,
handleCloseDrawer,
setHTMLUpdateAvailable
handleCloseDrawer
};
};
14 changes: 14 additions & 0 deletions packages/pwa-buildpack/lib/Utilities/getClientConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const webpack = require('webpack');
const WebpackAssetsManifest = require('webpack-assets-manifest');
const TerserPlugin = require('terser-webpack-plugin');

const ServiceWorkerPlugin = require('../WebpackTools/plugins/ServiceWorkerPlugin');
const PWADevServer = require('../WebpackTools/PWADevServer');
const RootComponentsPlugin = require('../WebpackTools/plugins/RootComponentsPlugin');
const UpwardIncludePlugin = require('../WebpackTools/plugins/UpwardIncludePlugin');
Expand Down Expand Up @@ -173,6 +174,19 @@ module.exports = async function({
}
});
}
}),
new ServiceWorkerPlugin({
mode,
paths,
injectManifest: true,
enableServiceWorkerDebugging: !!projectConfig.section(
'devServer'
).serviceWorkerEnabled,
injectManifestConfig: {
include: [/\.(?:css|js|html|svg)$/],
swSrc: './src/ServiceWorker/sw.js',
swDest: './sw.js'
}
})
],
devtool: 'source-map',
Expand Down
89 changes: 0 additions & 89 deletions packages/pwa-buildpack/lib/Utilities/getServiceWorkerConfig.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ test('produces a webpack config and friendly manifest plugin', async () => {
.statsAsDirectory()
.statsAsFile()
.productionEnvironment();
const { clientConfig: config } = await configureWebpack({ context: '.' });
const config = await configureWebpack({ context: '.' });
expect(config).toMatchObject({
context: '.',
mode: 'production',
Expand Down Expand Up @@ -150,7 +150,7 @@ test('works in developer mode from cli', async () => {
.statsAsDirectory()
.statsAsMissing()
.productionEnvironment();
const { clientConfig } = await configureWebpack({
const clientConfig = await configureWebpack({
context: '.',
env: { mode: 'development' }
});
Expand All @@ -163,7 +163,7 @@ test('works in developer mode from fallback', async () => {
.statsAsDirectory()
.statsAsMissing()
.devEnvironment();
const { clientConfig } = await configureWebpack({ context: '.' });
const clientConfig = await configureWebpack({ context: '.' });

expect(clientConfig).toHaveProperty('mode', 'development');
});
Expand Down Expand Up @@ -217,7 +217,7 @@ test('handles special flags', async () => {
// declare at least one argument or Tapable won't give us anything.
const specialFeaturesTap = jest.fn(x => x);
specialFeaturesHook.tap('configureWebpack.spec.js', specialFeaturesTap);
const { clientConfig } = await configureWebpack({
const clientConfig = await configureWebpack({
context: './fake/different/context',
vendor: ['jest'],
special
Expand Down
5 changes: 1 addition & 4 deletions packages/pwa-buildpack/lib/WebpackTools/configureWebpack.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const pkgDir = require('pkg-dir');

const loadEnvironment = require('../Utilities/loadEnvironment');
const getClientConfig = require('../Utilities/getClientConfig');
const getServiceWorkerConfig = require('../Utilities/getServiceWorkerConfig');
const BuildBus = require('../BuildBus');
const BuildBusPlugin = require('./plugins/BuildBusPlugin');

Expand Down Expand Up @@ -112,16 +111,14 @@ async function configureWebpack(options) {
stats
};

const serviceWorkerConfig = getServiceWorkerConfig(configOptions);

const clientConfig = await getClientConfig({
...configOptions,
vendor: options.vendor || []
});

clientConfig.plugins.unshift(new BuildBusPlugin(bus, busTrackingQueue));

return { clientConfig, serviceWorkerConfig };
return clientConfig;
}

module.exports = configureWebpack;
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,11 @@ class ServiceWorkerPlugin {
*/
apply(compiler) {
const { enableServiceWorkerDebugging, mode } = this.config;
compiler.hooks.afterEmit.tap('ServiceWorkerPlugin', () => {
if (mode === 'development' && !enableServiceWorkerDebugging) {
console.warn('Emitting no ServiceWorker in development mode.');
} else {
this.applyWorkbox(compiler);
}
});
if (mode === 'development' && !enableServiceWorkerDebugging) {
console.warn('Emitting no ServiceWorker in development mode.');
} else {
this.applyWorkbox(compiler);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ const WriteFileWebpackPlugin = require('write-file-webpack-plugin');

const ServiceWorkerPlugin = require('../ServiceWorkerPlugin');

const fakeTap = jest.fn();

const fakeCompiler = { hooks: { afterEmit: { tap: fakeTap } } };
const fakeCompiler = { hooks: {} };

beforeEach(() => {
WorkboxPlugin.GenerateSW.mockClear();
Expand Down Expand Up @@ -39,11 +37,7 @@ test('returns a valid Webpack plugin', () => {

plugin.apply(fakeCompiler);

fakeTap.mock.calls[0][1].call(plugin);

expect(plugin).toHaveProperty('apply', expect.any(Function));
expect(fakeTap.mock.calls[0][0]).toBe('ServiceWorkerPlugin');
expect(fakeTap.mock.calls[0][1]).toBeInstanceOf(Function);
});

test('.apply calls WorkboxPlugin.GenerateSW in prod', () => {
Expand All @@ -62,8 +56,6 @@ test('.apply calls WorkboxPlugin.GenerateSW in prod', () => {

plugin.apply(fakeCompiler);

fakeTap.mock.calls[0][1].call(plugin);

expect(WriteFileWebpackPlugin).not.toHaveBeenCalled();
expect(WorkboxPlugin.GenerateSW).toHaveBeenCalledWith(
expect.objectContaining({
Expand All @@ -87,8 +79,6 @@ test('.apply calls nothing but warns in console in dev', () => {

plugin.apply(fakeCompiler);

fakeTap.mock.calls[0][1].call(plugin);

expect(console.warn).toHaveBeenCalledWith(
expect.stringContaining(
`Emitting no ServiceWorker in development mode.`
Expand Down Expand Up @@ -122,8 +112,6 @@ test('.apply generates and writes out a serviceworker when enableServiceWorkerDe

plugin.apply(fakeCompiler);

fakeTap.mock.calls[0][1].call(plugin);

expect(WriteFileWebpackPlugin).toHaveBeenCalledWith(
expect.objectContaining({
test: expect.objectContaining({
Expand Down Expand Up @@ -166,8 +154,6 @@ test('.apply uses `InjectManifest` when `injectManifest` is `true`', () => {

plugin.apply(fakeCompiler);

fakeTap.mock.calls[0][1].call(plugin);

expect(WorkboxPlugin.InjectManifest).toHaveBeenCalledWith(
expect.objectContaining(injectManifestConfig)
);
Expand Down
4 changes: 2 additions & 2 deletions packages/pwa-buildpack/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@
"terser-webpack-plugin": "~1.2.3",
"wait-for-expect": "~1.2.0",
"webpack": "~4.38.0",
"workbox-webpack-plugin": "~4.2.0"
"workbox-webpack-plugin": "5.1.3"
},
"peerDependencies": {
"babel-loader": "~8.0.5",
"css-loader": "~2.1.1",
"terser-webpack-plugin": "~1.2.3",
"webpack": "~4.38.0",
"workbox-webpack-plugin": "~4.2.0"
"workbox-webpack-plugin": "5.1.3"
},
"engines": {
"node": ">=10.x",
Expand Down
3 changes: 1 addition & 2 deletions packages/venia-concept/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@
"lodash.over": "~4.7.0",
"memoize-one": "~5.0.0",
"memory-fs": "~0.4.1",
"node-html-parser": "~1.1.16",
"prettier": "~1.16.4",
"prop-types": "~15.7.2",
"react": "~16.9.0",
Expand All @@ -117,7 +116,7 @@
"webpack-bundle-analyzer": "~3.3.2",
"webpack-cli": "~3.2.3",
"webpack-dev-server": "~3.2.1",
"workbox-webpack-plugin": "~4.2.0"
"workbox-webpack-plugin": "5.1.3"
},
"optionalDependencies": {
"iltorb": "~2.0.0",
Expand Down
Loading