Merge pull request #30 from parcel-bundler/sources-tweaks

Fixes and improvements to sourcepath handling
parcel-bundler · Sep 6, 2020 · 5b5d5fe · 5b5d5fe
2 parents 62949c2 + f33f874
commit 5b5d5fe
Show file tree

Hide file tree

Showing 14 changed files with 181 additions and 114 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@parcel/source-map",
-  "version": "2.0.0-alpha.4.13",
+  "version": "2.0.0-alpha.4.14",
   "main": "./dist/node.js",
   "browser": "./dist/wasm-browser.js",
   "license": "MIT",
@@ -16,6 +16,7 @@
     "prebuild": "prebuildify -t 10.20.1 --napi --strip --tag-libc",
     "build:dev": "node-gyp rebuild --debug",
     "rebuild": "rm -rf build && yarn build:dev",
+    "rebuild-all": "yarn transpile && yarn compile-wasm && yarn rebuild",
     "install": "node-gyp-build",
     "prepublish": "npm run transpile",
     "lint": "prettier --write bench/run.js src/*.js",

diff --git a/src/SourceMap.js b/src/SourceMap.js
@@ -1,23 +1,42 @@
 // @flow
-import type { ParsedMap, VLQMap, SourceMapStringifyOptions, IndexedMapping } from './types';
+import type { ParsedMap, VLQMap, SourceMapStringifyOptions, IndexedMapping, GenerateEmptyMapOptions } from './types';
 
 import path from 'path';
-import { generateInlineMap, partialVlqMapToSourceMap } from './utils';
+import { generateInlineMap, partialVlqMapToSourceMap, relatifyPath, normalizePath } from './utils';
 
 export default class SourceMap {
   /**
    * @private
    */
   sourceMapInstance: any;
 
+  /**
+   * @private
+   */
+  projectRoot: string;
+
+  /**
+   * Construct a SourceMap instance
+   *
+   * @param projectRoot root directory of the project, this is to ensure all source paths are relative to this path
+   */
+  constructor(projectRoot: string = '/') {
+    this.projectRoot = normalizePath(projectRoot);
+  }
+
   /**
    * Generates an empty map from the provided fileName and sourceContent
    *
    * @param sourceName path of the source file
    * @param sourceContent content of the source file
    * @param lineOffset an offset that gets added to the sourceLine index of each mapping
    */
-  static generateEmptyMap(sourceName: string, sourceContent: string, lineOffset: number = 0): SourceMap {
+  static generateEmptyMap({
+    projectRoot,
+    sourceName,
+    sourceContent,
+    lineOffset = 0,
+  }: GenerateEmptyMapOptions): SourceMap {
     throw new Error('SourceMap.generateEmptyMap() must be implemented when extending SourceMap');
   }
 
@@ -37,21 +56,7 @@ export default class SourceMap {
    * Appends raw VLQ mappings to the sourcemaps
    */
   addRawMappings(map: VLQMap, lineOffset: number = 0, columnOffset: number = 0): SourceMap {
-    let { sourcesContent, sources = [], mappings, names = [] } = map;
-    if (!sourcesContent) {
-      sourcesContent = sources.map(() => '');
-    } else {
-      sourcesContent = sourcesContent.map((content) => (content ? content : ''));
-    }
-    this.sourceMapInstance.addRawMappings(
-      mappings,
-      sources,
-      sourcesContent.map((content) => (content ? content : '')),
-      names,
-      lineOffset,
-      columnOffset
-    );
-    return this;
+    throw new Error('SourceMap.addRawMappings() must be implemented when extending SourceMap');
   }
 
   /**
@@ -90,7 +95,7 @@ export default class SourceMap {
       hasValidOriginal ? mapping.original.line - 1 : -1,
       // $FlowFixMe
       hasValidOriginal ? mapping.original.column : -1,
-      mapping.source || '',
+      mapping.source ? relatifyPath(mapping.source, this.projectRoot) : '',
       mapping.name || ''
     );
   }
@@ -144,7 +149,7 @@ export default class SourceMap {
    * @returns the index of the added source filepath in the sources array
    */
   addSource(source: string): number {
-    return this.sourceMapInstance.addSource(source);
+    return this.sourceMapInstance.addSource(relatifyPath(source, this.projectRoot));
   }
 
   /**
@@ -163,7 +168,7 @@ export default class SourceMap {
    * @param source the filepath of the source file
    */
   getSourceIndex(source: string): number {
-    return this.sourceMapInstance.getSourceIndex(source);
+    return this.sourceMapInstance.getSourceIndex(relatifyPath(source, this.projectRoot));
   }
 
   /**
@@ -183,7 +188,7 @@ export default class SourceMap {
    * @param sourceContent the content of the sourceFile
    */
   setSourceContent(sourceName: string, sourceContent: string): void {
-    return this.sourceMapInstance.setSourceContent(sourceName, sourceContent);
+    return this.sourceMapInstance.setSourceContent(relatifyPath(sourceName, this.projectRoot), sourceContent);
   }
 
   /**
@@ -192,7 +197,7 @@ export default class SourceMap {
    * @param sourceName filename
    */
   getSourceContent(sourceName: string): string {
-    return this.sourceMapInstance.getSourceContent(sourceName);
+    return this.sourceMapInstance.getSourceContent(relatifyPath(sourceName, this.projectRoot));
   }
 
   /**
@@ -299,6 +304,9 @@ export default class SourceMap {
    * @param options options used for formatting the serialised map
    */
   async stringify(options: SourceMapStringifyOptions): Promise<string | VLQMap> {
-    return partialVlqMapToSourceMap(this.toVLQ(), options);
+    return partialVlqMapToSourceMap(this.toVLQ(), {
+      ...options,
+      rootDir: this.projectRoot || options.rootDir,
+    });
   }
 }
diff --git a/src/node.js b/src/node.js
@@ -1,16 +1,35 @@
 // @flow
-import type { ParsedMap, VLQMap, SourceMapStringifyOptions, IndexedMapping } from './types';
+import type { ParsedMap, VLQMap, SourceMapStringifyOptions, IndexedMapping, GenerateEmptyMapOptions } from './types';
 import path from 'path';
 import SourceMap from './SourceMap';
+import { relatifyPath } from './utils';
 
 const bindings = require('node-gyp-build')(path.join(__dirname, '..'));
 
 export default class NodeSourceMap extends SourceMap {
-  constructor() {
-    super();
+  constructor(projectRoot: string = '/') {
+    super(projectRoot);
     this.sourceMapInstance = new bindings.SourceMap();
   }
 
+  addRawMappings(map: VLQMap, lineOffset: number = 0, columnOffset: number = 0): SourceMap {
+    let { sourcesContent, sources = [], mappings, names = [] } = map;
+    if (!sourcesContent) {
+      sourcesContent = sources.map(() => '');
+    } else {
+      sourcesContent = sourcesContent.map((content) => (content ? content : ''));
+    }
+    this.sourceMapInstance.addRawMappings(
+      mappings,
+      sources.map((source) => (source ? relatifyPath(source, this.projectRoot) : '')),
+      sourcesContent.map((content) => (content ? content : '')),
+      names,
+      lineOffset,
+      columnOffset
+    );
+    return this;
+  }
+
   toBuffer(): Buffer {
     return this.sourceMapInstance.toBuffer();
   }
@@ -23,9 +42,14 @@ export default class NodeSourceMap extends SourceMap {
 
   delete() {}
 
-  static generateEmptyMap(sourceName: string, sourceContent: string, lineOffset: number = 0): NodeSourceMap {
-    let map = new NodeSourceMap();
-    map.addEmptyMap(sourceName, sourceContent, lineOffset);
+  static generateEmptyMap({
+    projectRoot,
+    sourceName,
+    sourceContent,
+    lineOffset = 0,
+  }: GenerateEmptyMapOptions): NodeSourceMap {
+    let map = new NodeSourceMap(projectRoot);
+    map.addEmptyMap(relatifyPath(sourceName, projectRoot), sourceContent, lineOffset);
     return map;
   }
 }

diff --git a/src/types.js b/src/types.js
@@ -33,9 +33,20 @@ export type VLQMap = {
 export type SourceMapStringifyOptions = {
   file?: string,
   sourceRoot?: string,
-  rootDir?: string,
   inlineSources?: boolean,
   fs?: { readFile(path: string, encoding: string): Promise<string>, ... },
   format?: 'inline' | 'string' | 'object',
+  /**
+   * @private
+   */
+  rootDir?: string,
+  ...
+};
+
+export type GenerateEmptyMapOptions = {
+  projectRoot: string,
+  sourceName: string,
+  sourceContent: string,
+  lineOffset?: number,
   ...
 };
diff --git a/src/utils.js b/src/utils.js
@@ -7,22 +7,24 @@ export function generateInlineMap(map: string): string {
   return `data:application/json;charset=utf-8;base64,${Buffer.from(map).toString('base64')}`;
 }
 
-function normalisePath(filepath: string): string {
-  return filepath.replace(/\\/g, '/');
+export function normalizePath(filepath: string): string {
+  filepath = filepath.replace(/\\/g, '/');
+
+  // Prefix relative paths with ./ as it makes it more clear and probably prevents issues
+  if (filepath.length > 0 && filepath[0] !== '.' && !path.isAbsolute(filepath)) {
+    filepath = `./${filepath}`;
+  }
+
+  return filepath;
 }
 
-function relatifyPath(filepath: string, rootDir: string): string {
+export function relatifyPath(filepath: string, rootDir: string): string {
   // Sourcemaps are made for web, so replace backslashes with regular slashes
-  filepath = normalisePath(filepath);
+  filepath = normalizePath(filepath);
 
   // Make root paths relative to the rootDir
   if (filepath[0] === '/') {
-    filepath = normalisePath(path.relative(rootDir, filepath));
-  }
-
-  // Prefix relative paths with ./ as it makes it more clear and probably prevents issues
-  if (filepath[0] !== '.') {
-    filepath = `./${filepath}`;
+    filepath = normalizePath(path.relative(rootDir, filepath));
   }
 
   return filepath;
@@ -32,7 +34,6 @@ export async function partialVlqMapToSourceMap(
   map: VLQMap,
   { fs, file, sourceRoot, inlineSources, rootDir, format = 'string' }: SourceMapStringifyOptions
 ): Promise<VLQMap | string> {
-  let root = normalisePath(rootDir || '/');
   let resultMap = {
     ...map,
     sourcesContent: map.sourcesContent ? map.sourcesContent.map((content) => (content ? content : null)) : [],
@@ -41,10 +42,6 @@ export async function partialVlqMapToSourceMap(
     sourceRoot,
   };
 
-  resultMap.sources = resultMap.sources.map((sourceFilePath) => {
-    return relatifyPath(sourceFilePath, root);
-  });
-
   if (resultMap.sourcesContent.length < resultMap.sources.length) {
     resultMap.sourcesContent.push(...new Array(resultMap.sources.length - resultMap.sourcesContent.length).fill(null));
   }
@@ -57,7 +54,7 @@ export async function partialVlqMapToSourceMap(
         // Because of this we have to include the sourceContent to ensure you can always see the sourcecontent for each mapping.
         if (!content && (inlineSources || sourceName.startsWith('..'))) {
           try {
-            return await fs.readFile(path.resolve(root, sourceName), 'utf-8');
+            return await fs.readFile(path.resolve(rootDir || '/', sourceName), 'utf-8');
           } catch (e) {}
         }
 

diff --git a/src/wasm.js b/src/wasm.js
@@ -1,7 +1,8 @@
 // @flow
-import type { ParsedMap, VLQMap, SourceMapStringifyOptions, IndexedMapping } from './types';
+import type { ParsedMap, VLQMap, SourceMapStringifyOptions, IndexedMapping, GenerateEmptyMapOptions } from './types';
 import path from 'path';
 import SourceMap from './SourceMap';
+import {relatifyPath} from './utils';
 
 let Module;
 
@@ -36,14 +37,19 @@ function arrayToEmbind(Type, from): any {
 }
 
 export default class WasmSourceMap extends SourceMap {
-  constructor() {
-    super();
+  constructor(projectRoot: string = '/') {
+    super(projectRoot);
     this.sourceMapInstance = new Module.SourceMap();
   }
 
-  static generateEmptyMap(sourceName: string, sourceContent: string, lineOffset: number = 0): WasmSourceMap {
-    let map = new WasmSourceMap();
-    map.addEmptyMap(sourceName, sourceContent, lineOffset);
+  static generateEmptyMap({
+    projectRoot,
+    sourceName,
+    sourceContent,
+    lineOffset = 0,
+  }: GenerateEmptyMapOptions): WasmSourceMap {
+    let map = new WasmSourceMap(projectRoot);
+    map.addEmptyMap(relatifyPath(sourceName, projectRoot), sourceContent, lineOffset);
     return map;
   }
 
@@ -53,6 +59,7 @@ export default class WasmSourceMap extends SourceMap {
     columnOffset: number = 0
   ) {
     let { sourcesContent, sources = [], mappings, names = [] } = map;
+    sources = sources.map((source) => (source ? relatifyPath(source, this.projectRoot) : ''));
     if (!sourcesContent) {
       sourcesContent = sources.map(() => '');
     } else {