From ce6eafeb73fe05dced76b5378b6fbe8c8cb9b352 Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Sat, 28 Aug 2021 18:33:08 -0300 Subject: [PATCH 01/10] refactor: redirect subsequent imports of "source-map-support" to this package --- source-map-support.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/source-map-support.js b/source-map-support.js index e2568d7..b035daf 100644 --- a/source-map-support.js +++ b/source-map-support.js @@ -555,6 +555,19 @@ exports.install = function(options) { } } + // Use dynamicRequire to avoid including in browser bundles + var Module = dynamicRequire(module, 'module'); + + // Redirect subsequent imports of "source-map-support" + // to this package + const originalRequire = Module.prototype.require; + Module.prototype.require = function requireProxy(id) { + if (id === 'source-map-support') { + id = '@cspotcode/source-map-support'; + } + return originalRequire.call(this, id); + } + // Allow sources to be found by methods other than reading the files // directly from disk. if (options.retrieveFile) { @@ -577,8 +590,6 @@ exports.install = function(options) { // Support runtime transpilers that include inline source maps if (options.hookRequire && !isInBrowser()) { - // Use dynamicRequire to avoid including in browser bundles - var Module = dynamicRequire(module, 'module'); var $compile = Module.prototype._compile; if (!$compile.__sourceMapSupport) { From 48d7b0ade3f6e9a0cf75e5ae75601734affe388e Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Sat, 28 Aug 2021 19:27:51 -0300 Subject: [PATCH 02/10] refactor: change implementation to hook on Module._resolveFilename --- source-map-support.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/source-map-support.js b/source-map-support.js index b035daf..e8acd7d 100644 --- a/source-map-support.js +++ b/source-map-support.js @@ -560,12 +560,12 @@ exports.install = function(options) { // Redirect subsequent imports of "source-map-support" // to this package - const originalRequire = Module.prototype.require; - Module.prototype.require = function requireProxy(id) { - if (id === 'source-map-support') { - id = '@cspotcode/source-map-support'; + const originalResolveFilename = Module._resolveFilename; + Module._resolveFilename = function resolveFilenameProxy(...args) { + if (args[0] === 'source-map-support') { + args[0] = '@cspotcode/source-map-support'; } - return originalRequire.call(this, id); + return originalResolveFilename.call(this, ...args); } // Allow sources to be found by methods other than reading the files From 1ebbd868ded45c2202cf176cd45560f0c9e531d2 Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Sun, 29 Aug 2021 13:04:00 -0300 Subject: [PATCH 03/10] refactor: list explicit arguments --- source-map-support.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source-map-support.js b/source-map-support.js index e8acd7d..4b9591a 100644 --- a/source-map-support.js +++ b/source-map-support.js @@ -561,11 +561,11 @@ exports.install = function(options) { // Redirect subsequent imports of "source-map-support" // to this package const originalResolveFilename = Module._resolveFilename; - Module._resolveFilename = function resolveFilenameProxy(...args) { - if (args[0] === 'source-map-support') { - args[0] = '@cspotcode/source-map-support'; + Module._resolveFilename = function resolveFilenameProxy(request, parent, isMain, options) { + if (request === 'source-map-support') { + request = '@cspotcode/source-map-support'; } - return originalResolveFilename.call(this, ...args); + return originalResolveFilename.call(this, request, parent, isMain, options); } // Allow sources to be found by methods other than reading the files From eb81879ba69e5c8b9be577a013087ed569631175 Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Sun, 29 Aug 2021 18:34:28 -0300 Subject: [PATCH 04/10] refactor: return full resolved path for this package proxy --- source-map-support.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source-map-support.js b/source-map-support.js index 4b9591a..6010e0d 100644 --- a/source-map-support.js +++ b/source-map-support.js @@ -563,7 +563,7 @@ exports.install = function(options) { const originalResolveFilename = Module._resolveFilename; Module._resolveFilename = function resolveFilenameProxy(request, parent, isMain, options) { if (request === 'source-map-support') { - request = '@cspotcode/source-map-support'; + request = require.resolve('@cspotcode/source-map-support'); } return originalResolveFilename.call(this, request, parent, isMain, options); } From 44c3bced1e7cfcef1be4131e5eb1500d2e6948bc Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Sun, 3 Oct 2021 23:56:57 -0400 Subject: [PATCH 05/10] Several changes: Allow installers to be notified upon redirection so they can log warnings to users Allow installers to opt-out of redirection Support both entrypoints: source-map-support and source-map-support/register --- source-map-support.d.ts | 9 +++++++++ source-map-support.js | 20 +++++++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/source-map-support.d.ts b/source-map-support.d.ts index d91e5b6..66604c2 100755 --- a/source-map-support.d.ts +++ b/source-map-support.d.ts @@ -31,6 +31,15 @@ export interface Options { overrideRetrieveSourceMap?: boolean | undefined; retrieveFile?(path: string): string; retrieveSourceMap?(source: string): UrlAndMap | null; + /** + * Set false to disable redirection of require / import `source-map-support` to `@cspotcode/source-map-support` + */ + redirectConflictingLibrary?: boolean; + /** + * Callback will be called every time we redirect due to `redirectConflictingLibrary` + * This allows consumers to log helpful warnings if they choose. + */ + onConflictingLibraryRedirect?: (request, parent, isMain, redirectedRequest) => void; } export interface Position { diff --git a/source-map-support.js b/source-map-support.js index 6010e0d..bb9f8eb 100644 --- a/source-map-support.js +++ b/source-map-support.js @@ -560,12 +560,22 @@ exports.install = function(options) { // Redirect subsequent imports of "source-map-support" // to this package - const originalResolveFilename = Module._resolveFilename; - Module._resolveFilename = function resolveFilenameProxy(request, parent, isMain, options) { - if (request === 'source-map-support') { - request = require.resolve('@cspotcode/source-map-support'); + const {redirectConflictingLibrary = true, onConflictingLibraryRedirect} = options; + if(redirectConflictingLibrary) { + const originalResolveFilename = Module._resolveFilename; + Module._resolveFilename = function resolveFilenameProxy(request, parent, isMain, options) { + // Match all source-map-support entrypoints: source-map-support, source-map-support/register + if (request === 'source-map-support') { + const newRequest = require.resolve('./'); + if(onConflictingLibraryRedirect) onConflictingLibraryRedirect(request, parent, isMain, options, newRequest); + request = newRequest; + } else if (request === 'source-map-support/register') { + const newRequest = require.resolve('./register'); + if(onConflictingLibraryRedirect) onConflictingLibraryRedirect(request, parent, isMain, options, newRequest); + request = newRequest; + } + return originalResolveFilename.call(this, request, parent, isMain, options); } - return originalResolveFilename.call(this, request, parent, isMain, options); } // Allow sources to be found by methods other than reading the files From e865d190efe80f2d939255f50e0e17e5e1076bf4 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Mon, 4 Oct 2021 00:22:28 -0400 Subject: [PATCH 06/10] Correct types; add tests --- source-map-support.d.ts | 3 ++- test.js | 36 +++++++++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/source-map-support.d.ts b/source-map-support.d.ts index 66604c2..ff3dce3 100755 --- a/source-map-support.d.ts +++ b/source-map-support.d.ts @@ -38,8 +38,9 @@ export interface Options { /** * Callback will be called every time we redirect due to `redirectConflictingLibrary` * This allows consumers to log helpful warnings if they choose. + * @param parent NodeJS.Module which made the require() or require.resolve() call */ - onConflictingLibraryRedirect?: (request, parent, isMain, redirectedRequest) => void; + onConflictingLibraryRedirect?: (request: string, parent: any, isMain: boolean, redirectedRequest: string) => void; } export interface Position { diff --git a/test.js b/test.js index 1de210f..378dc62 100644 --- a/test.js +++ b/test.js @@ -1,4 +1,5 @@ -require('./source-map-support').install({ +const underTest = require('./source-map-support'); +underTest.install({ emptyCacheBetweenOperations: true // Needed to be able to test for failure }); @@ -680,3 +681,36 @@ it('supports multiple instances', function(done) { /^ at foo \((?:.*[/\\])?.original2\.js:1:1\)$/ ]); }); + +describe('redirects require() of "source-map-support" to this module', function() { + it('redirects', function() { + assert.strictEqual(require.resolve('source-map-support'), require.resolve('.')); + assert.strictEqual(require.resolve('source-map-support/register'), require.resolve('./register')); + assert.strictEqual(require('source-map-support'), require('.')); + }); + + it('emits notifications', function() { + let onConflictingLibraryRedirectCalls = []; + let onConflictingLibraryRedirectCalls2 = []; + underTest.install({ + onConflictingLibraryRedirect(request, parent, isMain, redirectedRequest) { + onConflictingLibraryRedirectCalls.push([...arguments]); + } + }); + underTest.install({ + onConflictingLibraryRedirect(request, parent, isMain, redirectedRequest) { + onConflictingLibraryRedirectCalls2.push([...arguments]); + } + }); + require.resolve('source-map-support'); + assert.strictEqual(onConflictingLibraryRedirectCalls.length, 1); + assert.strictEqual(onConflictingLibraryRedirectCalls2.length, 1); + for(const args of [onConflictingLibraryRedirectCalls[0], onConflictingLibraryRedirectCalls2[0]]) { + const [request, parent, isMain, redirectedRequest] = args; + assert.strictEqual(request, 'source-map-support'); + assert.strictEqual(parent, module); + assert.strictEqual(isMain, false); + assert.strictEqual(redirectedRequest, require.resolve('.')); + } + }); +}); \ No newline at end of file From f5db819736aaa6ab190a19db4ace60b4dd223935 Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Mon, 4 Oct 2021 11:16:12 -0300 Subject: [PATCH 07/10] refactor: adjust install & uninstall to use sharedData --- source-map-support.js | 25 +++++++++++++++++++++---- test.js | 20 ++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/source-map-support.js b/source-map-support.js index 0d02933..e94606d 100644 --- a/source-map-support.js +++ b/source-map-support.js @@ -77,6 +77,8 @@ var sharedData = initializeSharedData({ errorPrepareStackTraceHook: undefined, /** @type {HookState} */ processEmitHook: undefined, + /** @type {HookState} */ + moduleResolveFilenameHook: undefined, // If true, the caches are reset before a stack trace formatting operation emptyCacheBetweenOperations: false, @@ -639,9 +641,14 @@ exports.install = function(options) { // Redirect subsequent imports of "source-map-support" // to this package const {redirectConflictingLibrary = true, onConflictingLibraryRedirect} = options; - if(redirectConflictingLibrary) { - const originalResolveFilename = Module._resolveFilename; - Module._resolveFilename = function resolveFilenameProxy(request, parent, isMain, options) { + if(redirectConflictingLibrary && !sharedData.moduleResolveFilenameHook) { + const originalValue = Module._resolveFilename; + sharedData.moduleResolveFilenameHook = { + enabled: true, + originalValue, + installedValue: undefined + } + Module._resolveFilename = sharedData.moduleResolveFilenameHook.installedValue = function (request, parent, isMain, options) { // Match all source-map-support entrypoints: source-map-support, source-map-support/register if (request === 'source-map-support') { const newRequest = require.resolve('./'); @@ -652,7 +659,7 @@ exports.install = function(options) { if(onConflictingLibraryRedirect) onConflictingLibraryRedirect(request, parent, isMain, options, newRequest); request = newRequest; } - return originalResolveFilename.call(this, request, parent, isMain, options); + return originalValue.call(this, request, parent, isMain, options); } } @@ -759,6 +766,16 @@ exports.uninstall = function() { } sharedData.errorPrepareStackTraceHook = undefined; } + if (sharedData.moduleResolveFilenameHook) { + // Disable behavior + sharedData.moduleResolveFilenameHook.enabled = false; + // If possible, remove our hook function. May not be possible if subsequent third-party hooks have wrapped around us. + var Module = dynamicRequire(module, 'module'); + if(Module._resolveFilename === sharedData.moduleResolveFilenameHook.installedValue) { + Module._resolveFilename = sharedData.moduleResolveFilenameHook.originalValue; + } + sharedData.moduleResolveFilenameHook = undefined; + } } exports.resetRetrieveHandlers = function() { diff --git a/test.js b/test.js index a386d81..638c947 100644 --- a/test.js +++ b/test.js @@ -1,8 +1,10 @@ // Note: some tests rely on side-effects from prior tests. // You may not get meaningful results running a subset of tests. +const Module = require('module'); const priorErrorPrepareStackTrace = Error.prepareStackTrace; const priorProcessEmit = process.emit; +const priorResolveFilename = Module._resolveFilename; const underTest = require('./source-map-support'); var SourceMapGenerator = require('source-map').SourceMapGenerator; var child_process = require('child_process'); @@ -742,11 +744,13 @@ describe('uninstall', function() { underTest.uninstall(); process.emit = priorProcessEmit; Error.prepareStackTrace = priorErrorPrepareStackTrace; + Module._resolveFilename = priorResolveFilename; }); it('uninstall removes hooks and source-mapping behavior', function() { assert.strictEqual(Error.prepareStackTrace, priorErrorPrepareStackTrace); assert.strictEqual(process.emit, priorProcessEmit); + assert.strictEqual(Module._resolveFilename, priorResolveFilename); normalThrowWithoutSourceMapSupportInstalled(); }); @@ -806,4 +810,20 @@ describe('uninstall', function() { process.emit('foo'); assert(peInvocations >= 1); }); + + it('uninstall preserves third-party module._resolveFilename hooks installed after us', function() { + installSms(); + const wrappedResolveFilename = Module._resolveFilename; + let peInvocations = 0; + function thirdPartyModuleResolveFilename() { + peInvocations++; + return wrappedResolveFilename.apply(this, arguments); + } + Module._resolveFilename = thirdPartyModuleResolveFilename; + underTest.uninstall(); + assert.strictEqual(Module._resolveFilename, thirdPartyModuleResolveFilename); + normalThrowWithoutSourceMapSupportInstalled(); + Module._resolveFilename('repl'); + assert(peInvocations >= 1); + }); }); From 226b96a021b2824b2a985909dd5eebddca2103ff Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Mon, 4 Oct 2021 17:42:58 -0300 Subject: [PATCH 08/10] refactor: add `onConflictingLibraryRedirect` array to notify all `install` of request redirect --- source-map-support.js | 49 ++++++++++++++++++++++++++----------------- test.js | 3 ++- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/source-map-support.js b/source-map-support.js index e94606d..a21469e 100644 --- a/source-map-support.js +++ b/source-map-support.js @@ -77,7 +77,7 @@ var sharedData = initializeSharedData({ errorPrepareStackTraceHook: undefined, /** @type {HookState} */ processEmitHook: undefined, - /** @type {HookState} */ + /** @type {HookState & { onConflictingLibraryRedirectArr: Array<(request: string, parent: any, isMain: boolean, redirectedRequest: string) => void> }} */ moduleResolveFilenameHook: undefined, // If true, the caches are reset before a stack trace formatting operation @@ -641,25 +641,36 @@ exports.install = function(options) { // Redirect subsequent imports of "source-map-support" // to this package const {redirectConflictingLibrary = true, onConflictingLibraryRedirect} = options; - if(redirectConflictingLibrary && !sharedData.moduleResolveFilenameHook) { - const originalValue = Module._resolveFilename; - sharedData.moduleResolveFilenameHook = { - enabled: true, - originalValue, - installedValue: undefined - } - Module._resolveFilename = sharedData.moduleResolveFilenameHook.installedValue = function (request, parent, isMain, options) { - // Match all source-map-support entrypoints: source-map-support, source-map-support/register - if (request === 'source-map-support') { - const newRequest = require.resolve('./'); - if(onConflictingLibraryRedirect) onConflictingLibraryRedirect(request, parent, isMain, options, newRequest); - request = newRequest; - } else if (request === 'source-map-support/register') { - const newRequest = require.resolve('./register'); - if(onConflictingLibraryRedirect) onConflictingLibraryRedirect(request, parent, isMain, options, newRequest); - request = newRequest; + if(redirectConflictingLibrary) { + if (!sharedData.moduleResolveFilenameHook) { + const originalValue = Module._resolveFilename; + sharedData.moduleResolveFilenameHook = { + enabled: true, + originalValue, + installedValue: undefined, + onConflictingLibraryRedirectArr: [] + } + Module._resolveFilename = sharedData.moduleResolveFilenameHook.installedValue = function (request, parent, isMain, options) { + // Match all source-map-support entrypoints: source-map-support, source-map-support/register + let requestRedirect; + if (request === 'source-map-support') { + requestRedirect = './'; + } else if (request === 'source-map-support/register') { + requestRedirect = './register'; + } + + if (requestRedirect !== undefined) { + const newRequest = require.resolve(requestRedirect); + for (const cb of sharedData.moduleResolveFilenameHook.onConflictingLibraryRedirectArr) { + cb(request, parent, isMain, options, newRequest); + } + request = newRequest; + } + return originalValue.call(this, request, parent, isMain, options); } - return originalValue.call(this, request, parent, isMain, options); + } + if (onConflictingLibraryRedirect) { + sharedData.moduleResolveFilenameHook.onConflictingLibraryRedirectArr.push(onConflictingLibraryRedirect); } } diff --git a/test.js b/test.js index 638c947..f57cf89 100644 --- a/test.js +++ b/test.js @@ -730,10 +730,11 @@ describe('redirects require() of "source-map-support" to this module', function( assert.strictEqual(onConflictingLibraryRedirectCalls.length, 1); assert.strictEqual(onConflictingLibraryRedirectCalls2.length, 1); for(const args of [onConflictingLibraryRedirectCalls[0], onConflictingLibraryRedirectCalls2[0]]) { - const [request, parent, isMain, redirectedRequest] = args; + const [request, parent, isMain, options, redirectedRequest] = args; assert.strictEqual(request, 'source-map-support'); assert.strictEqual(parent, module); assert.strictEqual(isMain, false); + assert.strictEqual(options, undefined); assert.strictEqual(redirectedRequest, require.resolve('.')); } }); From a0fdd9b99b4202e6d66ff3e65a34a7e5a7ce8e46 Mon Sep 17 00:00:00 2001 From: ejose19 <8742215+ejose19@users.noreply.github.com> Date: Tue, 5 Oct 2021 15:09:58 -0300 Subject: [PATCH 09/10] refactor: only apply redirect if the hook is enabled --- source-map-support.js | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/source-map-support.js b/source-map-support.js index a21469e..763e472 100644 --- a/source-map-support.js +++ b/source-map-support.js @@ -651,21 +651,24 @@ exports.install = function(options) { onConflictingLibraryRedirectArr: [] } Module._resolveFilename = sharedData.moduleResolveFilenameHook.installedValue = function (request, parent, isMain, options) { - // Match all source-map-support entrypoints: source-map-support, source-map-support/register - let requestRedirect; - if (request === 'source-map-support') { - requestRedirect = './'; - } else if (request === 'source-map-support/register') { - requestRedirect = './register'; - } + if (sharedData.moduleResolveFilenameHook && sharedData.moduleResolveFilenameHook.enabled) { + // Match all source-map-support entrypoints: source-map-support, source-map-support/register + let requestRedirect; + if (request === 'source-map-support') { + requestRedirect = './'; + } else if (request === 'source-map-support/register') { + requestRedirect = './register'; + } - if (requestRedirect !== undefined) { - const newRequest = require.resolve(requestRedirect); - for (const cb of sharedData.moduleResolveFilenameHook.onConflictingLibraryRedirectArr) { - cb(request, parent, isMain, options, newRequest); - } - request = newRequest; + if (requestRedirect !== undefined) { + const newRequest = require.resolve(requestRedirect); + for (const cb of sharedData.moduleResolveFilenameHook.onConflictingLibraryRedirectArr) { + cb(request, parent, isMain, options, newRequest); + } + request = newRequest; + } } + return originalValue.call(this, request, parent, isMain, options); } } From a4e164813f64c4f1c03385bbd94a16164e38aa32 Mon Sep 17 00:00:00 2001 From: Andrew Bradley Date: Tue, 5 Oct 2021 17:58:11 -0400 Subject: [PATCH 10/10] ensure disabled hook fn remains disabled when subsequent hook is installed on top if it --- source-map-support.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/source-map-support.js b/source-map-support.js index 763e472..268ccab 100644 --- a/source-map-support.js +++ b/source-map-support.js @@ -77,9 +77,12 @@ var sharedData = initializeSharedData({ errorPrepareStackTraceHook: undefined, /** @type {HookState} */ processEmitHook: undefined, - /** @type {HookState & { onConflictingLibraryRedirectArr: Array<(request: string, parent: any, isMain: boolean, redirectedRequest: string) => void> }} */ + /** @type {HookState} */ moduleResolveFilenameHook: undefined, + /** @type {Array<(request: string, parent: any, isMain: boolean, redirectedRequest: string) => void>} */ + onConflictingLibraryRedirectArr: [], + // If true, the caches are reset before a stack trace formatting operation emptyCacheBetweenOperations: false, @@ -644,14 +647,13 @@ exports.install = function(options) { if(redirectConflictingLibrary) { if (!sharedData.moduleResolveFilenameHook) { const originalValue = Module._resolveFilename; - sharedData.moduleResolveFilenameHook = { + const moduleResolveFilenameHook = sharedData.moduleResolveFilenameHook = { enabled: true, originalValue, installedValue: undefined, - onConflictingLibraryRedirectArr: [] } Module._resolveFilename = sharedData.moduleResolveFilenameHook.installedValue = function (request, parent, isMain, options) { - if (sharedData.moduleResolveFilenameHook && sharedData.moduleResolveFilenameHook.enabled) { + if (moduleResolveFilenameHook.enabled) { // Match all source-map-support entrypoints: source-map-support, source-map-support/register let requestRedirect; if (request === 'source-map-support') { @@ -662,7 +664,7 @@ exports.install = function(options) { if (requestRedirect !== undefined) { const newRequest = require.resolve(requestRedirect); - for (const cb of sharedData.moduleResolveFilenameHook.onConflictingLibraryRedirectArr) { + for (const cb of sharedData.onConflictingLibraryRedirectArr) { cb(request, parent, isMain, options, newRequest); } request = newRequest; @@ -673,7 +675,7 @@ exports.install = function(options) { } } if (onConflictingLibraryRedirect) { - sharedData.moduleResolveFilenameHook.onConflictingLibraryRedirectArr.push(onConflictingLibraryRedirect); + sharedData.onConflictingLibraryRedirectArr.push(onConflictingLibraryRedirect); } } @@ -790,6 +792,7 @@ exports.uninstall = function() { } sharedData.moduleResolveFilenameHook = undefined; } + sharedData.onConflictingLibraryRedirectArr.length = 0; } exports.resetRetrieveHandlers = function() {