From 0410c6f7145700774730f844c068798349837347 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 ++++++-- react-packager/src/Resolver/index.js | 46 +++++++++++++------ 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/react-packager/src/Resolver/__tests__/Resolver-test.js b/react-packager/src/Resolver/__tests__/Resolver-test.js index 18bf519d..684ef403 100644 --- a/react-packager/src/Resolver/__tests__/Resolver-test.js +++ b/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/react-packager/src/Resolver/index.js b/react-packager/src/Resolver/index.js index 87e23493..2300ee98 100644 --- a/react-packager/src/Resolver/index.js +++ b/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;