Really no sourcemaps (#4528)

parcel-bundler · Apr 30, 2020 · 8c43afe · 8c43afe
1 parent 619cbaf
commit 8c43afe
Show file tree

Hide file tree

Showing 9 changed files with 68 additions and 25 deletions.
diff --git a/packages/core/core/src/AssetGraph.js b/packages/core/core/src/AssetGraph.js
@@ -266,7 +266,14 @@ export default class AssetGraph extends Graph<AssetGraphNode> {
     let depNodesWithAssets = [];
     for (let dep of assetNode.value.dependencies.values()) {
       let depNode = nodeFromDep(dep);
-      depNodes.push(depNode);
+      let depNodeExisting = this.nodes.get(depNode.id);
+      if (depNodeExisting) {
+        invariant(depNodeExisting.type === 'dependency');
+        depNodeExisting.value = dep;
+        depNodes.push(depNodeExisting);
+      } else {
+        depNodes.push(depNode);
+      }
       let dependentAsset = dependentAssets.find(
         a => a.uniqueKey === dep.moduleSpecifier,
       );

diff --git a/packages/core/core/src/PackagerRunner.js b/packages/core/core/src/PackagerRunner.js
@@ -460,7 +460,12 @@ export default class PackagerRunner {
     };
 
     let mapKey = cacheKeys.map;
-    if (await this.options.cache.blobExists(mapKey)) {
+    if (
+      (typeof bundle.target.sourceMap === 'object'
+        ? !bundle.target.sourceMap.inline
+        : bundle.target.sourceMap) &&
+      (await this.options.cache.blobExists(mapKey))
+    ) {
       let mapStream = this.options.cache.getStream(mapKey);
       await writeFileStream(
         outputFS,

diff --git a/packages/core/core/src/TargetResolver.js b/packages/core/core/src/TargetResolver.js
@@ -139,7 +139,7 @@ export default class TargetResolver {
               scopeHoist:
                 this.options.scopeHoist && descriptor.scopeHoist !== false,
             }),
-            sourceMap: descriptor.sourceMap,
+            sourceMap: normalizeSourceMap(this.options, descriptor.sourceMap),
           };
         });
       }
@@ -351,7 +351,7 @@ export default class TargetResolver {
             scopeHoist:
               this.options.scopeHoist && descriptor.scopeHoist !== false,
           }),
-          sourceMap: descriptor.sourceMap,
+          sourceMap: normalizeSourceMap(this.options, descriptor.sourceMap),
           loc,
         });
       }
@@ -430,7 +430,7 @@ export default class TargetResolver {
             scopeHoist:
               this.options.scopeHoist && descriptor.scopeHoist !== false,
           }),
-          sourceMap: descriptor.sourceMap,
+          sourceMap: normalizeSourceMap(this.options, descriptor.sourceMap),
           loc,
         });
       }
@@ -450,6 +450,7 @@ export default class TargetResolver {
           minify: this.options.minify,
           scopeHoist: this.options.scopeHoist,
         }),
+        sourceMap: this.options.sourceMaps ? {} : undefined,
       });
     }
 
@@ -577,3 +578,15 @@ function assertNoDuplicateTargets(targets, pkgFilePath, pkgContents) {
     });
   }
 }
+
+function normalizeSourceMap(options: ParcelOptions, sourceMap) {
+  if (options.sourceMaps) {
+    if (typeof sourceMap === 'boolean') {
+      return sourceMap ? {} : undefined;
+    } else {
+      return sourceMap ?? {};
+    }
+  } else {
+    return undefined;
+  }
+}
diff --git a/packages/core/core/test/TargetResolver.test.js b/packages/core/core/test/TargetResolver.test.js
@@ -4,9 +4,10 @@ import assert from 'assert';
 import path from 'path';
 import tempy from 'tempy';
 import {inputFS as fs} from '@parcel/test-utils';
-import {DEFAULT_OPTIONS} from './utils';
-
 import TargetResolver from '../src/TargetResolver';
+import {DEFAULT_OPTIONS as _DEFAULT_OPTIONS} from './utils';
+
+const DEFAULT_OPTIONS = {..._DEFAULT_OPTIONS, sourceMaps: true};
 
 const COMMON_TARGETS_FIXTURE_PATH = path.join(
   __dirname,
@@ -87,7 +88,7 @@ describe('TargetResolver', () => {
               minify: false,
               scopeHoist: false,
             },
-            sourceMap: undefined,
+            sourceMap: {},
           },
           {
             name: 'customB',
@@ -104,7 +105,7 @@ describe('TargetResolver', () => {
               minify: false,
               scopeHoist: false,
             },
-            sourceMap: undefined,
+            sourceMap: {},
           },
         ],
       },
@@ -137,7 +138,7 @@ describe('TargetResolver', () => {
               minify: false,
               scopeHoist: false,
             },
-            sourceMap: undefined,
+            sourceMap: {},
             loc: {
               filePath: path.join(COMMON_TARGETS_FIXTURE_PATH, 'package.json'),
               start: {
@@ -203,7 +204,7 @@ describe('TargetResolver', () => {
               minify: false,
               scopeHoist: false,
             },
-            sourceMap: undefined,
+            sourceMap: {},
             loc: {
               filePath: path.join(COMMON_TARGETS_FIXTURE_PATH, 'package.json'),
               start: {
@@ -298,7 +299,7 @@ describe('TargetResolver', () => {
               minify: false,
               scopeHoist: false,
             },
-            sourceMap: undefined,
+            sourceMap: {},
             loc: {
               filePath: path.join(CUSTOM_TARGETS_FIXTURE_PATH, 'package.json'),
               start: {
@@ -330,7 +331,7 @@ describe('TargetResolver', () => {
               minify: false,
               scopeHoist: false,
             },
-            sourceMap: undefined,
+            sourceMap: {},
             loc: {
               filePath: path.join(CUSTOM_TARGETS_FIXTURE_PATH, 'package.json'),
               start: {
@@ -362,7 +363,7 @@ describe('TargetResolver', () => {
               minify: false,
               scopeHoist: false,
             },
-            sourceMap: undefined,
+            sourceMap: {},
             loc: {
               filePath: path.join(CUSTOM_TARGETS_FIXTURE_PATH, 'package.json'),
               start: {
@@ -406,7 +407,7 @@ describe('TargetResolver', () => {
             minify: false,
             scopeHoist: false,
           },
-          sourceMap: undefined,
+          sourceMap: {},
           loc: {
             filePath: path.join(CONTEXT_FIXTURE_PATH, 'package.json'),
             start: {
@@ -450,7 +451,7 @@ describe('TargetResolver', () => {
             minify: false,
             scopeHoist: false,
           },
-          sourceMap: undefined,
+          sourceMap: {},
           loc: {
             filePath: path.join(fixture, 'package.json'),
             start: {
@@ -496,7 +497,7 @@ describe('TargetResolver', () => {
               minify: false,
               scopeHoist: false,
             },
-            sourceMap: undefined,
+            sourceMap: {},
             loc: {
               filePath: path.join(COMMON_TARGETS_FIXTURE_PATH, 'package.json'),
               start: {
@@ -528,7 +529,7 @@ describe('TargetResolver', () => {
               minify: false,
               scopeHoist: false,
             },
-            sourceMap: undefined,
+            sourceMap: {},
             loc: {
               filePath: path.join(COMMON_TARGETS_FIXTURE_PATH, 'package.json'),
               start: {

diff --git a/packages/core/core/test/fixtures/common-targets-ignore/package.json b/packages/core/core/test/fixtures/common-targets-ignore/package.json
@@ -6,6 +6,8 @@
   },
   "targets": {
     "main": false,
-    "app": {}
+    "app": {
+      "sourceMap": false
+    }
   }
 }
diff --git a/packages/core/integration-tests/test/scope-hoisting.js b/packages/core/integration-tests/test/scope-hoisting.js
@@ -12,11 +12,10 @@ import {
   getNextBuild,
 } from '@parcel/test-utils';
 
-const bundle = (name, opts = {}) =>
-  _bundle(name, Object.assign({scopeHoist: true}, opts));
+const bundle = (name, opts = {}) => _bundle(name, {scopeHoist: true, ...opts});
 
 const bundler = (name, opts = {}) =>
-  _bundler(name, Object.assign({scopeHoist: true}, opts));
+  _bundler(name, {scopeHoist: true, ...opts});
 
 describe('scope hoisting', function() {
   describe('es6', function() {

diff --git a/packages/core/integration-tests/test/sourcemaps.js b/packages/core/integration-tests/test/sourcemaps.js
@@ -3,14 +3,16 @@ import path from 'path';
 import os from 'os';
 import SourceMap from '@parcel/source-map';
 import {
-  bundle,
+  bundle as _bundle,
   assertBundleTree,
   inputFS,
   outputFS,
   shallowEqual,
 } from '@parcel/test-utils';
 import {loadSourceMapUrl} from '@parcel/utils';
 
+const bundle = (name, opts = {}) => _bundle(name, {sourceMaps: true, ...opts});
+
 function indexToLineCol(str, index) {
   let beforeIndex = str.slice(0, index);
   return {
@@ -878,6 +880,20 @@ describe('sourcemaps', function() {
     assert.deepEqual(map.sources, ['index.js']);
   });
 
+  it('should respect --no-source-maps', async function() {
+    let b = await bundle(
+      path.join(__dirname, '/integration/sourcemap/index.js'),
+      {
+        sourceMaps: false,
+      },
+    );
+
+    assert.deepStrictEqual(
+      await outputFS.readdir(path.dirname(b.getBundles()[0].filePath)),
+      ['index.js'],
+    );
+  });
+
   it.skip('should load existing sourcemaps for CSS files', async function() {
     async function test(minify) {
       let b = await bundle(

diff --git a/packages/core/types/index.js b/packages/core/types/index.js
@@ -100,7 +100,7 @@ export type PackageTargetDescriptor = {|
   +outputFormat?: OutputFormat,
   +publicUrl?: string,
   +distDir?: FilePath,
-  +sourceMap?: TargetSourceMapOptions,
+  +sourceMap?: boolean | TargetSourceMapOptions,
   +isLibrary?: boolean,
   +minify?: boolean,
   +scopeHoist?: boolean,

diff --git a/packages/packagers/js/src/JSPackager.js b/packages/packagers/js/src/JSPackager.js
@@ -84,7 +84,7 @@ export default new Packager({
         queue.add(async () => {
           let [code, mapBuffer] = await Promise.all([
             node.value.getCode(),
-            node.value.getMapBuffer(),
+            bundle.target.sourceMap && node.value.getMapBuffer(),
           ]);
           return {code, mapBuffer};
         });