From 2cc587f3f410697f9e4e0a5e48af282fbdf21250 Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Mon, 19 Dec 2016 06:12:27 -0800 Subject: [PATCH] Improve source map quality when inserting numeric module IDs Summary: This changes the way that module identifiers are replaced with numeric ids so that ids are padded with spaces on the right to take up the same space as the original string literal. By doing this, we keep the mappings for the generated source line intact. ``` // original source const React = require('react'); // old replacement const React = require(12 /* react */); // new replacement const React = require(12 ); // 12 = react ``` The remaining edge case are module names that are replaced with numeric IDs that are longer than the replaced string, e.g.: ``` const Q = require('q'); const Q = require(1234); ``` Reviewed By: cpojer Differential Revision: D4346092 fbshipit-source-id: ef3bb879495f388c4a7448a2f810b83c204e8984 --- .../src/Resolver/__tests__/Resolver-test.js | 20 ++++++-- packager/react-packager/src/Resolver/index.js | 46 +++++++++++++------ 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/packager/react-packager/src/Resolver/__tests__/Resolver-test.js b/packager/react-packager/src/Resolver/__tests__/Resolver-test.js index 18bf519d04e..684ef403c56 100644 --- a/packager/react-packager/src/Resolver/__tests__/Resolver-test.js +++ b/packager/react-packager/src/Resolver/__tests__/Resolver-test.js @@ -330,7 +330,7 @@ describe('Resolver', function() { var code = [ // require 'require("x")', - 'require("y")', + 'require("y");require(\'abc\');', 'require( \'z\' )', 'require( "a")', 'require("b" )', @@ -356,14 +356,17 @@ describe('Resolver', function() { return [ ['x', createModule('changed')], ['y', createModule('Y')], + ['abc', createModule('abc')] ]; } const moduleIds = new Map( resolutionResponse .getResolvedDependencyPairs() - .map(([importId, module]) => - [importId, resolutionResponse.getModuleId(module)]) + .map(([importId, module]) => [ + importId, + padRight(resolutionResponse.getModuleId(module), importId.length + 2), + ]) ); return depResolver.wrapModule({ @@ -377,8 +380,9 @@ describe('Resolver', function() { expect(processedCode).toEqual([ '__d(/* test module */function(global, require, module, exports) {' + // require - `require(${moduleIds.get('x')} /* x */)`, - `require(${moduleIds.get('y')} /* y */)`, + `require(${moduleIds.get('x')}) // ${moduleIds.get('x').trim()} = x`, + `require(${moduleIds.get('y')});require(${moduleIds.get('abc') + }); // ${moduleIds.get('abc').trim()} = abc // ${moduleIds.get('y').trim()} = y`, 'require( \'z\' )', 'require( "a")', 'require("b" )', @@ -535,4 +539,10 @@ describe('Resolver', function() { return ({path}) => knownIds.get(path) || createId(path); } + + function padRight(value, width) { + const s = String(value); + const diff = width - s.length; + return diff > 0 ? s + Array(diff + 1).join(' ') : s; + } }); diff --git a/packager/react-packager/src/Resolver/index.js b/packager/react-packager/src/Resolver/index.js index 87e23493a55..2300ee989bc 100644 --- a/packager/react-packager/src/Resolver/index.js +++ b/packager/react-packager/src/Resolver/index.js @@ -226,21 +226,13 @@ class Resolver { // require('./c') => require(3); // -- in b/index.js: // require('../a/c') => require(3); - const replaceModuleId = (codeMatch, quote, depName) => - depName in resolvedDeps - ? `${JSON.stringify(resolvedDeps[depName])} /* ${depName} */` - : codeMatch; - - const codeParts = dependencyOffsets.reduceRight((codeBits, offset) => { - const first = codeBits.shift(); - codeBits.unshift( - first.slice(0, offset), - first.slice(offset).replace(/(['"])([^'"']*)\1/, replaceModuleId), - ); - return codeBits; - }, [code]); - - return codeParts.join(''); + return dependencyOffsets.reduceRight( + ([unhandled, handled], offset) => [ + unhandled.slice(0, offset), + replaceDependencyID(unhandled.slice(offset) + handled, resolvedDeps), + ], + [code, ''], + ).join(''); } wrapModule({ @@ -318,4 +310,28 @@ function definePolyfillCode(code,) { ].join(''); } +const reDepencencyString = /^(['"])([^'"']*)\1/; +function replaceDependencyID(stringWithDependencyIDAtStart, resolvedDeps) { + const match = reDepencencyString.exec(stringWithDependencyIDAtStart); + const dependencyName = match && match[2]; + if (match != null && dependencyName in resolvedDeps) { + const {length} = match[0]; + const id = String(resolvedDeps[dependencyName]); + return ( + padRight(id, length) + + stringWithDependencyIDAtStart + .slice(length) + .replace(/$/m, ` // ${id} = ${dependencyName}`) + ); + } else { + return stringWithDependencyIDAtStart; + } +} + +function padRight(string, length) { + return string.length < length + ? string + Array(length - string.length + 1).join(' ') + : string; +} + module.exports = Resolver;