From 99c114a5b4215d161326df8951edd52b52c163e5 Mon Sep 17 00:00:00 2001 From: DimitarTachev Date: Thu, 30 May 2019 17:59:21 +0300 Subject: [PATCH] fix: handle relative requests from inside an external module --- index.js | 22 ++++++- index.spec.ts | 107 ++++++++++++++++++++++++++++++- templates/webpack.angular.js | 2 +- templates/webpack.config.spec.ts | 30 +++++++-- templates/webpack.javascript.js | 2 +- templates/webpack.typescript.js | 2 +- templates/webpack.vue.js | 2 +- 7 files changed, 152 insertions(+), 15 deletions(-) diff --git a/index.js b/index.js index e76ce2cb..23992b2d 100644 --- a/index.js +++ b/index.js @@ -92,12 +92,32 @@ exports.getSourceMapFilename = (hiddenSourceMap, appFolderPath, outputPath) => { */ exports.getConvertedExternals = (externals) => { const modifiedExternals = (externals || []).map((e) => { - return new RegExp(`^${e}((/.*)|$)`); + return new RegExp(`((node_modules\/)|^)${e}((/.*)|$)`); }); return modifiedExternals; }; +exports.getExternalsHandler = (externalRegexps) => { + if (!Array.isArray(externalRegexps) || externalRegexps.some(regExp => !(regExp instanceof RegExp))) { + throw new Error(`Expected Array of regular expressions. Got '${externalRegexps}'.`); + } + + const handler = (context, request, callback) => { + let nonRelativeRequest = request; + if (request && request.startsWith(".")) { + nonRelativeRequest = path.join(context, request); + } + + if (externalRegexps.some(regExp => regExp.test(nonRelativeRequest))) { + return callback(null, 'commonjs ' + request); + } + callback(); + } + + return handler; +} + const sanitize = name => name .split("") .filter(char => /[a-zA-Z0-9]/.test(char)) diff --git a/index.spec.ts b/index.spec.ts index eda14f88..36affd3a 100644 --- a/index.spec.ts +++ b/index.spec.ts @@ -1,7 +1,107 @@ -import { getConvertedExternals, getEntryPathRegExp } from './index'; +import { getConvertedExternals, getEntryPathRegExp, getExternalsHandler } from './index'; const path = require("path"); describe('index', () => { + describe('getExternalsHandler', () => { + it('throws an error when the argument is not an array of regular expressions', () => { + expect(() => getExternalsHandler()).toThrowError(); + expect(() => getExternalsHandler(32)).toThrowError(); + expect(() => getExternalsHandler(null)).toThrowError(); + expect(() => getExternalsHandler("asd")).toThrowError(); + expect(() => getExternalsHandler([{}])).toThrowError(); + expect(() => getExternalsHandler(/((node_modules\/)|^)nativescript-vue((\/.*)|$)/)).toThrowError(); + }); + + const externalContextTestCases = [ + { + name: 'request for another external module from node_modules', + input: 'nativescript-angular', + expectedIsExternal: true + }, + { + name: 'request for the same module from node_modules', + input: 'nativescript-vue', + expectedIsExternal: true + }, + { + name: 'request for a relative file in the same external module', + input: './vue-index', + expectedIsExternal: true + }, + { + name: 'request for a relative file outside the external module', + input: '../another-non-external-module/another-index', + expectedIsExternal: false + }, + { + name: 'request for an absolute local path', + input: '/nativescript-vue', + expectedIsExternal: false + } + ]; + + externalContextTestCases.forEach(testCase => { + it(`returns handler which matches requests inside an external module - ${testCase.name}`, (done) => { + const handler = getExternalsHandler([/((node_modules\/)|^)nativescript-vue((\/.*)|$)/, /((node_modules\/)|^)nativescript-angular((\/.*)|$)/]); + handler("./node_modules/nativescript-vue/", testCase.input, (error, externalLink) => { + if (testCase.expectedIsExternal) { + expect(error).toBeNull(); + expect(externalLink).toBeDefined(); + } else { + expect(error).toBeUndefined(); + expect(externalLink).toBeUndefined(); + } + done(); + }); + }); + }); + + + const nonExternalContextTestCases = [ + { + name: 'request for an external module from node_modules', + input: 'nativescript-vue', + expectedIsExternal: true + }, + { + name: 'request for a relative file from an external module', + input: '../node_modules/nativescript-vue/vue-index', + expectedIsExternal: true + }, + { + name: 'request for a relative file inside the app', + input: './app-module', + expectedIsExternal: false + }, + { + name: 'request for a relative file from a non external module', + input: '../node_modules/another-non-external-module/another-index', + expectedIsExternal: false + }, + { + name: 'request for an absolute local path', + input: '/nativescript-vue', + expectedIsExternal: false + } + ]; + + nonExternalContextTestCases.forEach(testCase => { + it(`returns handler which matches requests from inside the app folder - ${testCase.name}`, (done) => { + const handler = getExternalsHandler([/((node_modules\/)|^)nativescript-vue((\/.*)|$)/]); + handler("./app/", testCase.input, (error, externalLink) => { + if (testCase.expectedIsExternal) { + expect(error).toBeNull(); + expect(externalLink).toBeDefined(); + } else { + expect(error).toBeUndefined(); + expect(externalLink).toBeUndefined(); + } + done(); + }); + }); + }); + }); + describe('getConvertedExternals', () => { it('returns empty array when nullable is passed', () => { const actualResult = getConvertedExternals(null); @@ -11,11 +111,11 @@ describe('index', () => { const testCases = [ { input: ['nativescript-vue'], - expectedOutput: [/^nativescript-vue((\/.*)|$)/] + expectedOutput: [/((node_modules\/)|^)nativescript-vue((\/.*)|$)/] }, { input: ['nativescript-vue', 'nativescript-angular'], - expectedOutput: [/^nativescript-vue((\/.*)|$)/, /^nativescript-angular((\/.*)|$)/] + expectedOutput: [/((node_modules\/)|^)nativescript-vue((\/.*)|$)/, /((node_modules\/)|^)nativescript-angular((\/.*)|$)/] } ]; @@ -29,6 +129,7 @@ describe('index', () => { [ 'nativescript-vue', 'nativescript-vue/subdir', + './node_modules/nativescript-vue/subdir', 'nativescript-vue/subdir/subdir-2' ].forEach(testString => { const regExpsExternals = getConvertedExternals(testCase.input); diff --git a/templates/webpack.angular.js b/templates/webpack.angular.js index a57f5cf9..c18fa51b 100644 --- a/templates/webpack.angular.js +++ b/templates/webpack.angular.js @@ -103,7 +103,7 @@ module.exports = env => { const config = { mode: uglify ? "production" : "development", context: appFullPath, - externals, + externals: nsWebpack.getExternalsHandler(externals), watchOptions: { ignored: [ appResourcesFullPath, diff --git a/templates/webpack.config.spec.ts b/templates/webpack.config.spec.ts index 155e7859..0d9eb9e1 100644 --- a/templates/webpack.config.spec.ts +++ b/templates/webpack.config.spec.ts @@ -23,6 +23,12 @@ class TerserJsStub { } }; +let providedExternalRegExps: any; +const fakeExternalsHandler = () => null; +const testGetExternalsHandler = (regExps) => { + providedExternalRegExps = regExps; + return fakeExternalsHandler; +}; const nativeScriptDevWebpack = { GenerateBundleStarterPlugin: EmptyClass, WatchStateLoggerPlugin: EmptyClass, @@ -32,6 +38,7 @@ const nativeScriptDevWebpack = { getResolver: () => null, getEntryPathRegExp: () => null, getConvertedExternals: nsWebpackIndex.getConvertedExternals, + getExternalsHandler: testGetExternalsHandler, getSourceMapFilename: nsWebpackIndex.getSourceMapFilename }; @@ -94,41 +101,50 @@ describe('webpack.config.js', () => { describe(`verify externals for webpack.${type}.js (${platform})`, () => { afterEach(() => { nativeScriptDevWebpack.getConvertedExternals = nsWebpackIndex.getConvertedExternals; + nativeScriptDevWebpack.getExternalsHandler = testGetExternalsHandler; }); it('returns empty array when externals are not passed', () => { const input = getInput({ platform }); const config = webpackConfig(input); - expect(config.externals).toEqual([]); + expect(config.externals).toEqual(fakeExternalsHandler); + expect(providedExternalRegExps).toEqual([]); }); it('calls getConvertedExternals to convert externals', () => { - let isCalled = false; + let isConvertExternalsCalled = false; + let isGetExternalsHandlerCalled = false; nativeScriptDevWebpack.getConvertedExternals = () => { - isCalled = true; + isConvertExternalsCalled = true; return []; }; + nativeScriptDevWebpack.getExternalsHandler = () => { + isGetExternalsHandlerCalled = true; + return () => null; + }; const input = getInput({ platform, externals: ['nativescript-vue'] }); webpackConfig(input); - expect(isCalled).toBe(true, 'Webpack.config.js must use the getConvertedExternals method'); + expect(isConvertExternalsCalled).toBe(true, 'Webpack.config.js must use the getConvertedExternals method'); + expect(isGetExternalsHandlerCalled).toBe(true, 'Webpack.config.js must use the getExternalsHandler method'); }); [ { input: ['nativescript-vue'], - expectedOutput: [/^nativescript-vue((\/.*)|$)/] + expectedOutput: [/((node_modules\/)|^)nativescript-vue((\/.*)|$)/] }, { input: ['nativescript-vue', 'nativescript-angular'], - expectedOutput: [/^nativescript-vue((\/.*)|$)/, /^nativescript-angular((\/.*)|$)/] + expectedOutput: [/((node_modules\/)|^)nativescript-vue((\/.*)|$)/, /((node_modules\/)|^)nativescript-angular((\/.*)|$)/] }, ].forEach(testCase => { const input = getInput({ platform, externals: testCase.input }); it(`are correct regular expressions, for input ${testCase.input}`, () => { const config = webpackConfig(input); - expect(config.externals).toEqual(testCase.expectedOutput); + expect(config.externals).toEqual(fakeExternalsHandler); + expect(providedExternalRegExps).toEqual(testCase.expectedOutput); }); }); }); diff --git a/templates/webpack.javascript.js b/templates/webpack.javascript.js index baf83484..4fd6dd07 100644 --- a/templates/webpack.javascript.js +++ b/templates/webpack.javascript.js @@ -63,7 +63,7 @@ module.exports = env => { const config = { mode: uglify ? "production" : "development", context: appFullPath, - externals, + externals: nsWebpack.getExternalsHandler(externals), watchOptions: { ignored: [ appResourcesFullPath, diff --git a/templates/webpack.typescript.js b/templates/webpack.typescript.js index 4a3bc35a..b3de7401 100644 --- a/templates/webpack.typescript.js +++ b/templates/webpack.typescript.js @@ -67,7 +67,7 @@ module.exports = env => { const config = { mode: uglify ? "production" : "development", context: appFullPath, - externals, + externals: nsWebpack.getExternalsHandler(externals), watchOptions: { ignored: [ appResourcesFullPath, diff --git a/templates/webpack.vue.js b/templates/webpack.vue.js index 66f13fa3..4a37b902 100644 --- a/templates/webpack.vue.js +++ b/templates/webpack.vue.js @@ -71,7 +71,7 @@ module.exports = env => { const config = { mode: mode, context: appFullPath, - externals, + externals: nsWebpack.getExternalsHandler(externals), watchOptions: { ignored: [ appResourcesFullPath,