From 05082b1aec1cd4b87b4dd60f79712035c29e5ac1 Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Mon, 29 Nov 2021 13:55:10 -0300 Subject: [PATCH] fix(webpack5): include hmr handling only when enabled (#9685) * fix(webpack): respect hmr flag * fix(webpack): ensure correct loader order is used * chore: cleanup Co-authored-by: Igor Randjelovic --- .../__snapshots__/angular.spec.ts.snap | 42 +++------ .../__snapshots__/base.spec.ts.snap | 50 ++++------- .../__snapshots__/javascript.spec.ts.snap | 84 ++++------------- .../__snapshots__/react.spec.ts.snap | 90 ++++++++----------- .../__snapshots__/svelte.spec.ts.snap | 50 ++++------- .../__snapshots__/typescript.spec.ts.snap | 84 ++++------------- .../__snapshots__/vue.spec.ts.snap | 50 ++++------- packages/webpack5/src/configuration/base.ts | 30 ++++--- .../webpack5/src/configuration/javascript.ts | 27 +++--- .../webpack5/src/configuration/typescript.ts | 27 +++--- 10 files changed, 190 insertions(+), 344 deletions(-) diff --git a/packages/webpack5/__tests__/configuration/__snapshots__/angular.spec.ts.snap b/packages/webpack5/__tests__/configuration/__snapshots__/angular.spec.ts.snap index c888c2aad..0e9e5f8a2 100644 --- a/packages/webpack5/__tests__/configuration/__snapshots__/angular.spec.ts.snap +++ b/packages/webpack5/__tests__/configuration/__snapshots__/angular.spec.ts.snap @@ -84,23 +84,9 @@ exports[`angular configuration for android 1`] = ` options: { platform: 'android' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ - { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } } ] }, - /* config.module.rule('js') */ - { - test: /\\\\.js$/, - exclude: [ - /node_modules/ - ] - }, /* config.module.rule('workers') */ { test: /\\\\.(js|ts)$/, @@ -111,6 +97,13 @@ exports[`angular configuration for android 1`] = ` } ] }, + /* config.module.rule('js') */ + { + test: /\\\\.js$/, + exclude: [ + /node_modules/ + ] + }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -472,23 +465,9 @@ exports[`angular configuration for ios 1`] = ` options: { platform: 'ios' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ - { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } } ] }, - /* config.module.rule('js') */ - { - test: /\\\\.js$/, - exclude: [ - /node_modules/ - ] - }, /* config.module.rule('workers') */ { test: /\\\\.(js|ts)$/, @@ -499,6 +478,13 @@ exports[`angular configuration for ios 1`] = ` } ] }, + /* config.module.rule('js') */ + { + test: /\\\\.js$/, + exclude: [ + /node_modules/ + ] + }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, diff --git a/packages/webpack5/__tests__/configuration/__snapshots__/base.spec.ts.snap b/packages/webpack5/__tests__/configuration/__snapshots__/base.spec.ts.snap index 9209dd850..07ce49456 100644 --- a/packages/webpack5/__tests__/configuration/__snapshots__/base.spec.ts.snap +++ b/packages/webpack5/__tests__/configuration/__snapshots__/base.spec.ts.snap @@ -72,13 +72,16 @@ exports[`base configuration for android 1`] = ` options: { platform: 'android' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -110,16 +113,6 @@ exports[`base configuration for android 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -383,13 +376,16 @@ exports[`base configuration for ios 1`] = ` options: { platform: 'ios' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -421,16 +417,6 @@ exports[`base configuration for ios 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, diff --git a/packages/webpack5/__tests__/configuration/__snapshots__/javascript.spec.ts.snap b/packages/webpack5/__tests__/configuration/__snapshots__/javascript.spec.ts.snap index 8e642f69f..4347d13a6 100644 --- a/packages/webpack5/__tests__/configuration/__snapshots__/javascript.spec.ts.snap +++ b/packages/webpack5/__tests__/configuration/__snapshots__/javascript.spec.ts.snap @@ -72,13 +72,16 @@ exports[`javascript configuration for android 1`] = ` options: { platform: 'android' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -110,16 +113,6 @@ exports[`javascript configuration for android 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -183,23 +176,6 @@ exports[`javascript configuration for android 1`] = ` loader: 'sass-loader' } ] - }, - /* config.module.rule('hmr-core') */ - { - test: /\\\\.js$/, - exclude: [ - /node_modules/, - '__jest__/src/app.js' - ], - use: [ - /* config.module.rule('hmr-core').use('nativescript-hot-loader') */ - { - loader: 'nativescript-hot-loader', - options: { - appPath: '__jest__/src' - } - } - ] } ] }, @@ -409,13 +385,16 @@ exports[`javascript configuration for ios 1`] = ` options: { platform: 'ios' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -447,16 +426,6 @@ exports[`javascript configuration for ios 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -520,23 +489,6 @@ exports[`javascript configuration for ios 1`] = ` loader: 'sass-loader' } ] - }, - /* config.module.rule('hmr-core') */ - { - test: /\\\\.js$/, - exclude: [ - /node_modules/, - '__jest__/src/app.js' - ], - use: [ - /* config.module.rule('hmr-core').use('nativescript-hot-loader') */ - { - loader: 'nativescript-hot-loader', - options: { - appPath: '__jest__/src' - } - } - ] } ] }, diff --git a/packages/webpack5/__tests__/configuration/__snapshots__/react.spec.ts.snap b/packages/webpack5/__tests__/configuration/__snapshots__/react.spec.ts.snap index 102570df6..c875a21e6 100644 --- a/packages/webpack5/__tests__/configuration/__snapshots__/react.spec.ts.snap +++ b/packages/webpack5/__tests__/configuration/__snapshots__/react.spec.ts.snap @@ -85,6 +85,16 @@ exports[`react configuration > android > adds ReactRefreshWebpackPlugin when HMR } ] }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ + { + loader: 'nativescript-worker-loader' + } + ] + }, /* config.module.rule('ts') */ { test: [ @@ -125,16 +135,6 @@ exports[`react configuration > android > adds ReactRefreshWebpackPlugin when HMR /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -408,13 +408,16 @@ exports[`react configuration > android > base config 1`] = ` options: { platform: 'android' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -447,16 +450,6 @@ exports[`react configuration > android > base config 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -729,6 +722,16 @@ exports[`react configuration > ios > adds ReactRefreshWebpackPlugin when HMR ena } ] }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ + { + loader: 'nativescript-worker-loader' + } + ] + }, /* config.module.rule('ts') */ { test: [ @@ -769,16 +772,6 @@ exports[`react configuration > ios > adds ReactRefreshWebpackPlugin when HMR ena /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -1053,13 +1046,16 @@ exports[`react configuration > ios > base config 1`] = ` options: { platform: 'ios' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -1092,16 +1088,6 @@ exports[`react configuration > ios > base config 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, diff --git a/packages/webpack5/__tests__/configuration/__snapshots__/svelte.spec.ts.snap b/packages/webpack5/__tests__/configuration/__snapshots__/svelte.spec.ts.snap index 7e56d0469..1e311c402 100644 --- a/packages/webpack5/__tests__/configuration/__snapshots__/svelte.spec.ts.snap +++ b/packages/webpack5/__tests__/configuration/__snapshots__/svelte.spec.ts.snap @@ -76,13 +76,16 @@ exports[`svelte configuration for android 1`] = ` options: { platform: 'android' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts|svelte)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -114,16 +117,6 @@ exports[`svelte configuration for android 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts|svelte)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -408,13 +401,16 @@ exports[`svelte configuration for ios 1`] = ` options: { platform: 'ios' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts|svelte)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -446,16 +442,6 @@ exports[`svelte configuration for ios 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts|svelte)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, diff --git a/packages/webpack5/__tests__/configuration/__snapshots__/typescript.spec.ts.snap b/packages/webpack5/__tests__/configuration/__snapshots__/typescript.spec.ts.snap index c3ccd94a5..f8807fd5a 100644 --- a/packages/webpack5/__tests__/configuration/__snapshots__/typescript.spec.ts.snap +++ b/packages/webpack5/__tests__/configuration/__snapshots__/typescript.spec.ts.snap @@ -72,13 +72,16 @@ exports[`typescript configuration for android 1`] = ` options: { platform: 'android' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -110,16 +113,6 @@ exports[`typescript configuration for android 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -183,23 +176,6 @@ exports[`typescript configuration for android 1`] = ` loader: 'sass-loader' } ] - }, - /* config.module.rule('hmr-core') */ - { - test: /\\\\.(js|ts)$/, - exclude: [ - /node_modules/, - '__jest__/src/app.js' - ], - use: [ - /* config.module.rule('hmr-core').use('nativescript-hot-loader') */ - { - loader: 'nativescript-hot-loader', - options: { - appPath: '__jest__/src' - } - } - ] } ] }, @@ -409,13 +385,16 @@ exports[`typescript configuration for ios 1`] = ` options: { platform: 'ios' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -447,16 +426,6 @@ exports[`typescript configuration for ios 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -520,23 +489,6 @@ exports[`typescript configuration for ios 1`] = ` loader: 'sass-loader' } ] - }, - /* config.module.rule('hmr-core') */ - { - test: /\\\\.(js|ts)$/, - exclude: [ - /node_modules/, - '__jest__/src/app.js' - ], - use: [ - /* config.module.rule('hmr-core').use('nativescript-hot-loader') */ - { - loader: 'nativescript-hot-loader', - options: { - appPath: '__jest__/src' - } - } - ] } ] }, diff --git a/packages/webpack5/__tests__/configuration/__snapshots__/vue.spec.ts.snap b/packages/webpack5/__tests__/configuration/__snapshots__/vue.spec.ts.snap index 4282ea2d1..ce9e6407f 100644 --- a/packages/webpack5/__tests__/configuration/__snapshots__/vue.spec.ts.snap +++ b/packages/webpack5/__tests__/configuration/__snapshots__/vue.spec.ts.snap @@ -75,13 +75,16 @@ exports[`vue configuration for android 1`] = ` options: { platform: 'android' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -116,16 +119,6 @@ exports[`vue configuration for android 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, @@ -420,13 +413,16 @@ exports[`vue configuration for ios 1`] = ` options: { platform: 'ios' } - }, - /* config.module.rule('bundle').use('nativescript-hot-loader') */ + } + ] + }, + /* config.module.rule('workers') */ + { + test: /\\\\.(js|ts)$/, + use: [ + /* config.module.rule('workers').use('nativescript-worker-loader') */ { - loader: 'nativescript-hot-loader', - options: { - injectHMRRuntime: true - } + loader: 'nativescript-worker-loader' } ] }, @@ -461,16 +457,6 @@ exports[`vue configuration for ios 1`] = ` /node_modules/ ] }, - /* config.module.rule('workers') */ - { - test: /\\\\.(js|ts)$/, - use: [ - /* config.module.rule('workers').use('nativescript-worker-loader') */ - { - loader: 'nativescript-worker-loader' - } - ] - }, /* config.module.rule('xml') */ { test: /\\\\.xml$/, diff --git a/packages/webpack5/src/configuration/base.ts b/packages/webpack5/src/configuration/base.ts index 37d85c86d..e618b5d68 100644 --- a/packages/webpack5/src/configuration/base.ts +++ b/packages/webpack5/src/configuration/base.ts @@ -198,12 +198,24 @@ export default function (config: Config, env: IWebpackEnv = _env): Config { .options({ platform, }) - .end() - .use('nativescript-hot-loader') - .loader('nativescript-hot-loader') - .options({ - injectHMRRuntime: true, - }); + .end(); + + config.when(env.hmr, (config) => { + config.module + .rule('bundle') + .use('nativescript-hot-loader') + .loader('nativescript-hot-loader') + .options({ + injectHMRRuntime: true, + }); + }); + + // worker-loader should be declared before ts-loader + config.module + .rule('workers') + .test(/\.(js|ts)$/) + .use('nativescript-worker-loader') + .loader('nativescript-worker-loader'); // set up ts support config.module @@ -249,12 +261,6 @@ export default function (config: Config, env: IWebpackEnv = _env): Config { .exclude.add(/node_modules/) .end(); - config.module - .rule('workers') - .test(/\.(js|ts)$/) - .use('nativescript-worker-loader') - .loader('nativescript-worker-loader'); - // config.resolve.extensions.add('.xml'); // set up xml config.module diff --git a/packages/webpack5/src/configuration/javascript.ts b/packages/webpack5/src/configuration/javascript.ts index 0696e9b7b..ae9eb3ff6 100644 --- a/packages/webpack5/src/configuration/javascript.ts +++ b/packages/webpack5/src/configuration/javascript.ts @@ -34,18 +34,21 @@ export default function (config: Config, env: IWebpackEnv = _env): Config { virtualEntryPath ); - // set up core HMR - config.module - .rule('hmr-core') - .test(/\.js$/) - .exclude.add(/node_modules/) - .add(entryPath) - .end() - .use('nativescript-hot-loader') - .loader('nativescript-hot-loader') - .options({ - appPath: getEntryDirPath(), - }); + config.when(env.hmr, (config) => { + // set up core HMR + config.module + .rule('hmr-core') + .before('js') + .test(/\.js$/) + .exclude.add(/node_modules/) + .add(entryPath) + .end() + .use('nativescript-hot-loader') + .loader('nativescript-hot-loader') + .options({ + appPath: getEntryDirPath(), + }); + }); return config; } diff --git a/packages/webpack5/src/configuration/typescript.ts b/packages/webpack5/src/configuration/typescript.ts index b8b9492a2..539471b03 100644 --- a/packages/webpack5/src/configuration/typescript.ts +++ b/packages/webpack5/src/configuration/typescript.ts @@ -34,18 +34,21 @@ export default function (config: Config, env: IWebpackEnv = _env): Config { virtualEntryPath ); - // set up core HMR - config.module - .rule('hmr-core') - .test(/\.(js|ts)$/) - .exclude.add(/node_modules/) - .add(entryPath) - .end() - .use('nativescript-hot-loader') - .loader('nativescript-hot-loader') - .options({ - appPath: getEntryDirPath(), - }); + config.when(env.hmr, (config) => { + // set up core HMR + config.module + .rule('hmr-core') + .before('ts') + .test(/\.(js|ts)$/) + .exclude.add(/node_modules/) + .add(entryPath) + .end() + .use('nativescript-hot-loader') + .loader('nativescript-hot-loader') + .options({ + appPath: getEntryDirPath(), + }); + }); return config; }