fix: refactor to fix dep map overwrite problem

An issue potentially existed where if multiple versions of the same
dependency existed with different file paths then when they were
inserted into the dependency map the last entry would win.

This change makes it so that individual feed items are handled
in isolation therefore avoiding a shared map which risks overwrite.

Author: digitalsadhu

lib/calculate-relative-path.js

@@ -0,0 +1,10 @@
+'use strict';
+
+const { join, relative, dirname, basename } = require('path');
+
+module.exports = function calculateRelativePath(feedItemPath, dependencyPath) {
+    const path = dirname(dependencyPath);
+    const filename = basename(dependencyPath);
+    const relativePath = relative(dirname(feedItemPath), path);
+    return `./${join(relativePath, filename)}`;
+};

lib/map-dep-names-to-paths.js

@@ -0,0 +1,19 @@
+'use strict';
+
+const calculateRelativePath = require('./calculate-relative-path');
+
+module.exports = function mapDepNamesToPaths(hashToFileMap, feedItem) {
+    const { deps } = feedItem;
+    const depToFileMap = new Map();
+    for (const dependency of Object.keys(deps)) {
+        if (['.', '/'].includes(dependency[0])) continue;
+        const dependencyId = deps[dependency];
+        const dependencyPath = hashToFileMap.get(dependencyId);
+        const relativePath = calculateRelativePath(
+            feedItem.file,
+            dependencyPath
+        );
+        depToFileMap.set(dependency, relativePath);
+    }
+    return depToFileMap;
+};

lib/map-feed-dependencies.js

@@ -0,0 +1,9 @@
+'use strict';
+
+module.exports = function mapHashToPath(feed) {
+    const hashToFileMap = new Map();
+    for (const item of feed) {
+        hashToFileMap.set(item.id, item.file);
+    }
+    return hashToFileMap;
+};

lib/map-hash-to-path.js

@@ -0,0 +1,14 @@
+'use strict';
+
+module.exports = function replaceRequireStatements(sourceCode, dependencyMap) {
+    for (const dependency of dependencyMap.keys()) {
+        // if relative or absolute path already then skip.
+        if (['.', '/'].includes(dependency[0])) continue;
+
+        sourceCode = sourceCode.replace(
+            new RegExp(`require\\s*\\(\\s*['"]${dependency}['"]\\s*\\)`, 'g'),
+            `require('${dependencyMap.get(dependency)}')`
+        );
+    }
+    return sourceCode;
+};

lib/replace-dependencies.js

@@ -2,8 +2,9 @@
 
 const { join } = require('path');
 const { outputFile } = require('fs-extra');
-const replaceDependencies = require('./replace-dependencies');
-const mapFeedDependencies = require('./map-feed-dependencies');
+const mapHashToPath = require('./map-hash-to-path');
+const mapDepNamesToPaths = require('./map-dep-names-to-paths');
+const replaceRequireStatements = require('./replace-require-statements');
 
 /**
  * Unpacks a given feed's source code. Recreates the directory structure
@@ -14,9 +15,11 @@ const mapFeedDependencies = require('./map-feed-dependencies');
  * @param {object[]} feed - browserify bundle feed
  */
 module.exports = async function unpackFeed(root, feed) {
-    const feedDependencies = mapFeedDependencies(feed);
+    const hashToFile = mapHashToPath(feed);
+
     for (const item of feed) {
-        const content = replaceDependencies(feedDependencies, item);
+        const depToFileMap = mapDepNamesToPaths(hashToFile, item);
+        const content = replaceRequireStatements(item.source, depToFileMap);
         await outputFile(join(root, item.file), content);
     }
 };

lib/replace-require-statements.js

@@ -0,0 +1,14 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`returns map of feed ids -> feed file paths 1`] = `
+Array [
+  Array [
+    "3c4ca5444d554f82cb15c2437ed545b2ea6f6a6e98476ec372b21b6cf7d8085a",
+    "/Users/ricwalke/src/asset-pipe/asset-pipe-js-reader/test/mock/podletA/entrypoint.js",
+  ],
+  Array [
+    "94def8251ca7023be6c5aeda886a4055d4d47fd62238228def5888398bbdf318",
+    "/Users/ricwalke/src/asset-pipe/asset-pipe-js-reader/test/mock/podletA/lib/index.js",
+  ],
+]
+`;

lib/unpack-feed.js

@@ -0,0 +1,27 @@
+'use strict';
+
+const calculateRelativePath = require('../lib/calculate-relative-path');
+
+test('divergent paths', () => {
+    const feedItemPath = '/common/path/another-dep/index.js';
+    const dependencyPath = '/common/path/my-dep/index.js';
+
+    const result = calculateRelativePath(feedItemPath, dependencyPath);
+    expect(result).toBe('./../my-dep/index.js');
+});
+
+test('nested dependency path', () => {
+    const feedItemPath = '/common/index.js';
+    const dependencyPath = '/common/dep/path/index.js';
+
+    const result = calculateRelativePath(feedItemPath, dependencyPath);
+    expect(result).toBe('./dep/path/index.js');
+});
+
+test('nested path -> parent path', () => {
+    const feedItemPath = '/common/dep/path/my-dep/index.js';
+    const dependencyPath = '/common/index.js';
+
+    const result = calculateRelativePath(feedItemPath, dependencyPath);
+    expect(result).toBe('./../../../index.js');
+});

test/__snapshots__/map-hash-to-path.test.js.snap

@@ -0,0 +1,73 @@
+'use strict';
+
+const mapDepNamesToPaths = require('../lib/map-dep-names-to-paths');
+
+test('dependency map created for feed item with single dep', () => {
+    const hashToFileMap = new Map([[1, '/common/one/index.js']]);
+    const item = {
+        id: 4,
+        file: '/common/index.js',
+        source: '',
+        deps: { one: 1 },
+    };
+
+    const result = mapDepNamesToPaths(hashToFileMap, item);
+    expect(result.get('one')).toBe('./one/index.js');
+    expect(Array.from(result)).toHaveLength(1);
+});
+
+test('dependency map created for feed item with multiple deps', () => {
+    const hashToFileMap = new Map([
+        [1, '/common/one/index.js'],
+        [2, '/common/two/index.js'],
+        [3, '/common/three/index.js'],
+    ]);
+    const item = {
+        id: 4,
+        file: '/common/index.js',
+        source: '',
+        deps: { one: 1, two: 2, three: 3 },
+    };
+
+    const result = mapDepNamesToPaths(hashToFileMap, item);
+    expect(result.get('one')).toBe('./one/index.js');
+    expect(result.get('two')).toBe('./two/index.js');
+    expect(result.get('three')).toBe('./three/index.js');
+    expect(Array.from(result)).toHaveLength(3);
+});
+
+test('relative path dependencies ignored', () => {
+    const hashToFileMap = new Map([
+        [1, '/common/one/index.js'],
+        [2, '/common/two/index.js'],
+        [3, '/common/three/index.js'],
+    ]);
+    const item = {
+        id: 4,
+        file: '/common/index.js',
+        source: '',
+        deps: { './one': 2 },
+    };
+
+    const result = mapDepNamesToPaths(hashToFileMap, item);
+    expect(result.has('./one')).toBe(false);
+    expect(Array.from(result)).toHaveLength(0);
+});
+
+test('absolute path dependencies ignored', () => {
+    const hashToFileMap = new Map([
+        [1, '/common/one/index.js'],
+        [2, '/common/two/index.js'],
+        [3, '/common/three/index.js'],
+    ]);
+    const item = {
+        id: 4,
+        file: '/common/index.js',
+        source: '',
+        deps: { '/one': 2 },
+    };
+
+    const result = mapDepNamesToPaths(hashToFileMap, item);
+    expect(result.has('/one')).toBe(false);
+    expect(Array.from(result)).toHaveLength(0);
+});