From 95ec10b7acecce2df94db53f77d605f316a6f6ed Mon Sep 17 00:00:00 2001 From: rosen-vladimirov Date: Mon, 9 Sep 2019 11:35:33 +0300 Subject: [PATCH] fix: handle webpack source maps when library is set In case the `library` path is set in webpack.config.js file, the URLs in sourceMaps are generated in a way that is not expected for our parser - currently we expect `webpack:///`, while it becomes `webpack:///`. To handle this, add the correct sourceMapPathOverrides in case the user had not specified it already. Add unit tests and add as devDependency `@types/sinon`. However, the project cannot be transpiled with the current TypeScript version (2.6) when this package is installed, so update TypeScript to latest version. Use proxyquire in the tests to mock the require of the webpack.config.js file. --- package.json | 9 ++- src/debug-adapter/nativeScriptDebugAdapter.ts | 27 ++++++-- src/tests/nativeScriptDebugAdapter.tests.ts | 67 +++++++++++++++++-- .../nativeScriptTargetDiscovery.tests.ts | 2 +- tslint.json | 1 - 5 files changed, 92 insertions(+), 14 deletions(-) diff --git a/package.json b/package.json index b87e9cc..53f23b3 100644 --- a/package.json +++ b/package.json @@ -36,14 +36,17 @@ "@types/lodash": "4.14.121", "@types/mocha": "5.2.6", "@types/node": "6.0.46", + "@types/proxyquire": "1.3.28", "@types/semver": "5.5.0", + "@types/sinon": "7.0.13", "@types/universal-analytics": "0.4.1", "cpx": "1.5.0", "mocha": "5.2.0", - "sinon": "5.1.1", - "tslint": "5.10.0", + "proxyquire": "2.1.3", + "sinon": "7.4.2", + "tslint": "5.19.0", "tslint-eslint-rules": "5.4.0", - "typescript": "2.6.2", + "typescript": "3.6.2", "vsce": "1.57.1", "vscode": "1.1.30", "vscode-debugprotocol": "1.34.0" diff --git a/src/debug-adapter/nativeScriptDebugAdapter.ts b/src/debug-adapter/nativeScriptDebugAdapter.ts index 033f379..3347986 100644 --- a/src/debug-adapter/nativeScriptDebugAdapter.ts +++ b/src/debug-adapter/nativeScriptDebugAdapter.ts @@ -8,7 +8,7 @@ import { ISetBreakpointsResponseBody, ITelemetryPropertyCollector, } from 'vscode-chrome-debug-core'; -import { Event, TerminatedEvent } from 'vscode-debugadapter'; +import { Event, logger, TerminatedEvent } from 'vscode-debugadapter'; import { DebugProtocol } from 'vscode-debugprotocol'; import * as extProtocol from '../common/extensionProtocol'; import { NativeScriptSourceMapTransformer } from './nativeScriptSourceMapTransformer'; @@ -179,13 +179,32 @@ export class NativeScriptDebugAdapter extends ChromeDebugAdapter { args.sourceMapPathOverrides = {}; } - if (!args.sourceMapPathOverrides['webpack:///*']) { - const appDirPath = this.getAppDirPath(args.webRoot) || 'app'; - const fullAppDirPath = join(args.webRoot, appDirPath); + const appDirPath = this.getAppDirPath(args.webRoot) || 'app'; + const fullAppDirPath = join(args.webRoot, appDirPath); + if (!args.sourceMapPathOverrides['webpack:///*']) { args.sourceMapPathOverrides['webpack:///*'] = `${fullAppDirPath}/*`; } + const webpackConfigFile = join(`./${args.webRoot}`, 'webpack.config.js'); + + if (existsSync(webpackConfigFile)) { + try { + const webpackConfig = require(webpackConfigFile); + const platform = args.platform && args.platform.toLowerCase(); + const config = webpackConfig({ [`${platform}`]: platform }); + + if (config && config.output && config.output.library) { + const sourceMapPathOverrideWithLib = `webpack://${config.output.library}/*`; + + args.sourceMapPathOverrides[sourceMapPathOverrideWithLib] = args.sourceMapPathOverrides[sourceMapPathOverrideWithLib] || + `${fullAppDirPath}/*`; + } + } catch (err) { + logger.warn(`Error when trying to require webpack.config.js file from path '${webpackConfigFile}'. Error is: ${err}`); + } + } + return args; } diff --git a/src/tests/nativeScriptDebugAdapter.tests.ts b/src/tests/nativeScriptDebugAdapter.tests.ts index 1f7e626..8328c2f 100644 --- a/src/tests/nativeScriptDebugAdapter.tests.ts +++ b/src/tests/nativeScriptDebugAdapter.tests.ts @@ -1,9 +1,18 @@ +import * as fs from 'fs'; import * as _ from 'lodash'; +import { join } from 'path'; +import * as proxyquire from 'proxyquire'; import * as sinon from 'sinon'; import { ChromeDebugAdapter } from 'vscode-chrome-debug-core'; import { Event } from 'vscode-debugadapter'; import * as extProtocol from '../common/extensionProtocol'; -import { NativeScriptDebugAdapter } from '../debug-adapter/nativeScriptDebugAdapter'; +const appRoot = 'appRootMock'; +const webpackConfigFunctionStub = sinon.stub(); + +proxyquire.noCallThru(); +const nativeScriptDebugAdapterLib = proxyquire('../debug-adapter/nativeScriptDebugAdapter', { + [join(appRoot, 'webpack.config.js')]: webpackConfigFunctionStub, +}); const examplePort = 456; @@ -17,7 +26,7 @@ const customMessagesResponses = { }; const defaultArgsMock: any = { - appRoot: 'appRootMock', + appRoot, diagnosticLogging: true, platform: 'android', request: 'attach', @@ -67,7 +76,7 @@ describe('NativeScriptDebugAdapter', () => { setTransformOptions: () => undefined, }; - nativeScriptDebugAdapter = new NativeScriptDebugAdapter({ + nativeScriptDebugAdapter = new nativeScriptDebugAdapterLib.NativeScriptDebugAdapter({ chromeConnection: mockConstructor(chromeConnectionMock), pathTransformer: mockConstructor(pathTransformerMock), sourceMapTransformer: mockConstructor(sourceMapTransformer), @@ -83,7 +92,11 @@ describe('NativeScriptDebugAdapter', () => { platforms.forEach((platform) => { launchMethods.forEach((method) => { - const argsMock = _.merge({}, defaultArgsMock, { platform, request: method }); + let argsMock: any; + + beforeEach(() => { + argsMock = _.merge({}, defaultArgsMock, { platform, request: method }); + }); it(`${method} for ${platform} should raise debug start event`, async () => { const spy = sinon.spy(chromeSessionMock, 'sendEvent'); @@ -121,7 +134,51 @@ describe('NativeScriptDebugAdapter', () => { sinon.assert.calledWith(spy, sinon.match({ trace: true, - webRoot: 'appRootMock', + webRoot: appRoot, + })); + }); + + it(`${method} for ${platform} should add sourceMapPathOverrides data`, async () => { + const spy = sinon.spy(ChromeDebugAdapter.prototype, 'attach'); + const existsSyncStub = sinon.stub(fs, 'existsSync'); + + existsSyncStub.returns(true); + webpackConfigFunctionStub + .withArgs({ [platform]: platform }) + .returns({ output: { library: 'myLib' } }); + + await nativeScriptDebugAdapter[method](argsMock); + + existsSyncStub.restore(); + sinon.assert.calledWith(spy, sinon.match({ + sourceMapPathOverrides: { + 'webpack:///*': `${join(appRoot, 'app')}/*`, + 'webpack://myLib/*': `${join(appRoot, 'app')}/*`, + }, + trace: true, + webRoot: appRoot, + })); + + }); + + it(`${method} for ${platform} should not fail when unable to require webpack.config.js`, async () => { + const spy = sinon.spy(ChromeDebugAdapter.prototype, 'attach'); + const existsSyncStub = sinon.stub(fs, 'existsSync'); + + existsSyncStub.returns(true); + webpackConfigFunctionStub + .withArgs({ [platform]: platform }) + .throws(new Error('test')); + + await nativeScriptDebugAdapter[method](argsMock); + + existsSyncStub.restore(); + sinon.assert.calledWith(spy, sinon.match({ + sourceMapPathOverrides: { + 'webpack:///*': `${join(appRoot, 'app')}/*`, + }, + trace: true, + webRoot: appRoot, })); }); diff --git a/src/tests/nativeScriptTargetDiscovery.tests.ts b/src/tests/nativeScriptTargetDiscovery.tests.ts index 18d6749..6e94bf1 100644 --- a/src/tests/nativeScriptTargetDiscovery.tests.ts +++ b/src/tests/nativeScriptTargetDiscovery.tests.ts @@ -21,7 +21,7 @@ describe('NativeScriptTargetDiscovery', () => { }); it(`getTargets calls getTarget`, async () => { - const testTarget = { + const testTarget: any = { devtoolsFrontendUrl: 'url', webSocketDebuggerUrl: 'socket', }; diff --git a/tslint.json b/tslint.json index b994373..79a0d19 100644 --- a/tslint.json +++ b/tslint.json @@ -27,7 +27,6 @@ "check-type" ], "newline-before-return": true, - "no-unused-variable": true, "no-duplicate-variable": true, "no-unused-expression": [ true,