Don't inline env vars & fs (parcel-bundler#4993)

packages/bundlers/default/src/DefaultBundler.js

@@ -518,6 +518,7 @@ const CONFIG_SCHEMA: SchemaEntity = {
       type: 'number',
     },
   },
+  additionalProperties: false,
 };
 
 async function loadBundlerConfig(options: PluginOptions) {

packages/core/core/src/TargetDescriptor.schema.js

@@ -97,6 +97,9 @@ export const DESCRIPTOR_SCHEMA: SchemaEntity = {
     minify: {
       type: 'boolean',
     },
+    scopeHoist: {
+      type: 'boolean',
+    },
   },
   additionalProperties: false,
 };

packages/core/integration-tests/test/blob-url.js

@@ -2,20 +2,11 @@
 
 import assert from 'assert';
 import path from 'path';
-import {bundle, distDir, outputFS, inputFS} from '@parcel/test-utils';
-
-const configPath = path.join(__dirname, '/integration/blob-url/.parcelrc');
-
-const config = {
-  ...JSON.parse(inputFS.readFileSync(configPath, 'utf8')),
-  filePath: configPath,
-};
+import {bundle, distDir, outputFS} from '@parcel/test-utils';
 
 describe('blob urls', () => {
   it('should inline compiled content as a blob url with `blob-url:*` imports', async () => {
-    await bundle(path.join(__dirname, '/integration/blob-url/index.js'), {
-      config,
-    });
+    await bundle(path.join(__dirname, '/integration/blob-url/index.js'));
 
     let bundleContent = await outputFS.readFile(
       path.join(distDir, 'index.js'),
@@ -36,7 +27,6 @@ describe('blob urls', () => {
 
   it('should inline minified content as a blob url with `blob-url:*` imports', async () => {
     await bundle(path.join(__dirname, '/integration/blob-url/index.js'), {
-      config,
       minify: true,
     });
 

packages/core/integration-tests/test/fs.js

@@ -1,10 +1,12 @@
+// @flow
 import assert from 'assert';
 import path from 'path';
 import {
   assertBundles,
   bundle,
   removeDistDirectory,
   run,
+  overlayFS,
   outputFS,
   distDir,
 } from '@parcel/test-utils';
@@ -15,6 +17,36 @@ describe('fs', function() {
   });
 
   describe('browser environment', function() {
+    it('should not inline a file if disabled via config', async function() {
+      let b = await bundle(
+        path.join(__dirname, '/integration/fs-disabled/index.js'),
+        {
+          inputFS: overlayFS,
+        },
+      );
+
+      // $FlowFixMe
+      await assert.rejects(() => run(b), {
+        message: `ENOENT: no such file or directory, open '...'`,
+        code: 'ENOENT',
+      });
+    });
+
+    it('should not inline a file outside of the project root', async function() {
+      let b = await bundle(
+        path.join(__dirname, '/integration/fs-outside-root/index.js'),
+        {
+          inputFS: overlayFS,
+        },
+      );
+
+      // $FlowFixMe
+      await assert.rejects(() => run(b), {
+        message: `ENOENT: no such file or directory, open '...'`,
+        code: 'ENOENT',
+      });
+    });
+
     it('should inline a file as a string', async function() {
       let b = await bundle(path.join(__dirname, '/integration/fs/index.js'));
       let output = await run(b);
@@ -198,7 +230,7 @@ describe('fs', function() {
   describe.skip('electron environment', function() {
     it('should not inline a file in an Electron environment', async function() {
       let b = await bundle(path.join(__dirname, '/integration/fs/index.js'), {
-        target: 'electron',
+        targets: ['electron'],
       });
 
       assertBundles(b, [

packages/core/integration-tests/test/integration/blob-url/package.json

@@ -0,0 +1 @@
+{}

packages/core/integration-tests/test/integration/env-disabled-glob/index.js

@@ -0,0 +1,3 @@
+module.exports = function () {
+  return process.env.A_1 + ":" + process.env.B_1 + ":" + process.env.B_2;
+};

packages/core/integration-tests/test/integration/env-disabled-glob/package.json

@@ -0,0 +1,5 @@
+{
+  "@parcel/transformer-js": {
+    "inlineEnvironment": ["B_*"]
+  }
+}

packages/core/integration-tests/test/integration/env-disabled/index.js

@@ -0,0 +1,7 @@
+module.exports = function () {
+  return process.env.FOOBAR + test(process.env.FOOBAR);
+};
+
+function test(str) {
+  return ':' + str;
+}

packages/core/integration-tests/test/integration/env-disabled/package.json

@@ -0,0 +1,5 @@
+{
+  "@parcel/transformer-js": {
+    "inlineEnvironment": false
+  }
+}

packages/core/integration-tests/test/integration/env-nodeenv/index.js

@@ -0,0 +1,3 @@
+module.exports = function () {
+  return process.env.NODE_ENV + ":" + process.env.FOO;
+};

packages/core/integration-tests/test/integration/env-nodeenv/package.json

@@ -0,0 +1,5 @@
+{
+  "@parcel/transformer-js": {
+    "inlineEnvironment": false
+  }
+}

packages/core/integration-tests/test/integration/env-prior-transform/insert-env.js

@@ -1,5 +1,5 @@
 module.exports = function insertEnv(babel) {
-  const exportFoo = babel.template('module.exports = process.env.foo;');
+  const exportFoo = babel.template('module.exports = process.env.NODE_ENV;', {syntacticPlaceholders: true});
 
   return {
     visitor: {

packages/core/integration-tests/test/integration/fs-disabled/index.js

@@ -0,0 +1,2 @@
+var fs = require('fs');
+module.exports = fs.readFileSync(__dirname + '/test.txt', 'utf8');

packages/core/integration-tests/test/integration/fs-disabled/package.json

@@ -0,0 +1,5 @@
+{
+  "@parcel/transformer-js": {
+    "inlineFS": false
+  }
+}

packages/core/integration-tests/test/integration/fs-disabled/test.txt

@@ -0,0 +1 @@
+hello

packages/core/integration-tests/test/integration/fs-outside-root/index.js

@@ -0,0 +1,4 @@
+var fs = require('fs');
+
+fs.readFileSync(__dirname + '../../../../../../../package.json', 'utf8')
+

packages/core/integration-tests/test/javascript.js

@@ -1238,10 +1238,10 @@ describe('javascript', function() {
     );
 
     let jsBundle = b.getBundles()[0];
-    let contents = await outputFS.readFile(jsBundle.filePath);
+    let contents = await outputFS.readFile(jsBundle.filePath, 'utf8');
 
     assert(!contents.includes('process.env'));
-    assert.equal(await run(b), 42);
+    assert.equal(await run(b), 'test');
   });
 
   it('should not insert environment variables in node environment', async function() {
@@ -1250,7 +1250,7 @@ describe('javascript', function() {
     );
 
     let output = await run(b);
-    assert.ok(output.toString().indexOf('process.env') > -1);
+    assert.ok(output.toString().includes('process.env'));
     assert.equal(output(), 'test:test');
   });
 
@@ -1265,7 +1265,7 @@ describe('javascript', function() {
     });
 
     let output = await run(b);
-    assert.ok(output.toString().indexOf('process.env') > -1);
+    assert.ok(output.toString().includes('process.env'));
     assert.equal(output(), 'test:test');
   });
 
@@ -1280,16 +1280,59 @@ describe('javascript', function() {
     });
 
     let output = await run(b);
-    assert.ok(output.toString().indexOf('process.env') > -1);
+    assert.ok(output.toString().includes('process.env'));
     assert.equal(output(), 'test:test');
   });
 
-  it('should insert environment variables in browser environment', async function() {
-    let b = await bundle(path.join(__dirname, '/integration/env/index.js'));
+  it('should inline NODE_ENV environment variable in browser environment even if disabled', async function() {
+    let b = await bundle(
+      path.join(__dirname, '/integration/env-nodeenv/index.js'),
+      {
+        env: {
+          FOO: 'abc',
+        },
+      },
+    );
 
     let output = await run(b);
-    assert.ok(output.toString().indexOf('process.env') === -1);
-    assert.equal(output(), 'test:test');
+    assert.ok(!output.toString().includes('process.env'));
+    assert.equal(output(), 'test:undefined');
+  });
+
+  it('should not insert environment variables in browser environment if disabled', async function() {
+    let b = await bundle(
+      path.join(__dirname, '/integration/env-disabled/index.js'),
+      {
+        env: {FOOBAR: 'abc'},
+      },
+    );
+
+    let output = await run(b);
+    assert.ok(!output.toString().includes('process.env'));
+    assert.equal(output(), 'undefined:undefined');
+  });
+
+  it('should only insert environment variables in browser environment matching the glob', async function() {
+    let b = await bundle(
+      path.join(__dirname, '/integration/env-disabled-glob/index.js'),
+      {
+        env: {A_1: 'abc', B_1: 'def', B_2: 'ghi'},
+      },
+    );
+
+    let output = await run(b);
+    assert.ok(!output.toString().includes('process.env'));
+    assert.equal(output(), 'undefined:def:ghi');
+  });
+
+  it('should be able to inline environment variables in browser environment', async function() {
+    let b = await bundle(path.join(__dirname, '/integration/env/index.js'), {
+      env: {NODE_ENV: 'abc'},
+    });
+
+    let output = await run(b);
+    assert.ok(!output.toString().includes('process.env'));
+    assert.equal(output(), 'abc:abc');
   });
 
   it("should insert the user's NODE_ENV as process.env.NODE_ENV if passed", async function() {
@@ -1300,7 +1343,7 @@ describe('javascript', function() {
     });
 
     let output = await run(b);
-    assert.ok(output.toString().indexOf('process.env') === -1);
+    assert.ok(!output.toString().includes('process.env'));
     assert.equal(output(), 'production:production');
   });
 

packages/core/test-utils/src/utils.js

@@ -245,7 +245,7 @@ export async function run(
   bundleGraph: BundleGraph<NamedBundle>,
   globals: mixed,
   opts: RunOpts = {},
-): Promise<mixed> {
+): Promise<any> {
   let bundles = bundleGraph.getBundles();
   let bundle = nullthrows(
     bundles.find(b => b.type === 'js' || b.type === 'html'),

packages/shared/babel-ast-utils/src/index.js

@@ -1,7 +1,14 @@
 // @flow strict-local
 
-import type {AST, BaseAsset, PluginOptions} from '@parcel/types';
+import type {
+  AST,
+  BaseAsset,
+  PluginOptions,
+  SourceLocation,
+} from '@parcel/types';
+import type {SourceLocation as BabelSourceLocation} from '@babel/types';
 
+import path from 'path';
 import babelGenerate from '@babel/generator';
 import {parse as babelParse} from '@babel/parser';
 import SourceMap from '@parcel/source-map';
@@ -73,3 +80,23 @@ export async function generate({
     map,
   };
 }
+
+export function convertBabelLoc(loc: ?BabelSourceLocation): ?SourceLocation {
+  if (!loc) return null;
+  let {filename, start, end} = loc;
+  if (filename == null) return null;
+  return {
+    filePath: path.normalize(filename),
+    start: {
+      line: start.line,
+      column: start.column + 1,
+    },
+    // - Babel's columns are exclusive, ours are inclusive (column - 1)
+    // - Babel has 0-based columns, ours are 1-based (column + 1)
+    // = +-0
+    end: {
+      line: end.line,
+      column: end.column,
+    },
+  };
+}

packages/shared/scope-hoisting/package.json

@@ -25,6 +25,7 @@
     "@babel/template": "^7.2.2",
     "@babel/traverse": "^7.2.3",
     "@babel/types": "^7.3.3",
+    "@parcel/babel-ast-utils": "2.0.0-beta.1",
     "@parcel/babylon-walk": "2.0.0-beta.1",
     "@parcel/diagnostic": "2.0.0-beta.1",
     "@parcel/source-map": "2.0.0-alpha.4.13",

packages/shared/scope-hoisting/src/hoist.js

@@ -35,13 +35,9 @@ import traverse from '@babel/traverse';
 import template from '@babel/template';
 import nullthrows from 'nullthrows';
 import invariant from 'assert';
+import {convertBabelLoc} from '@parcel/babel-ast-utils';
 import rename from './renamer';
-import {
-  convertBabelLoc,
-  getName,
-  getIdentifier,
-  getExportIdentifier,
-} from './utils';
+import {getName, getIdentifier, getExportIdentifier} from './utils';
 
 const WRAPPER_TEMPLATE = template.statement<
   {|NAME: LVal, BODY: Array<Statement>|},

packages/shared/scope-hoisting/src/link.js

@@ -36,11 +36,11 @@ import {
   isSequenceExpression,
   isStringLiteral,
 } from '@babel/types';
+import {convertBabelLoc} from '@parcel/babel-ast-utils';
 import traverse from '@babel/traverse';
 import treeShake from './shake';
 import {
   assertString,
-  convertBabelLoc,
   getName,
   getIdentifier,
   getThrowableDiagnosticForNode,
@@ -525,7 +525,7 @@ export function link({
             throw getThrowableDiagnosticForNode(
               "`require.resolve` calls for excluded assets are only supported with outputFormat: 'commonjs'",
               mapped.filePath,
-              path.node.loc,
+              convertBabelLoc(path.node.loc),
             );
           }
 
@@ -536,7 +536,7 @@ export function link({
           throw getThrowableDiagnosticForNode(
             "`require.resolve` calls for bundled modules or bundled assets aren't supported with scope hoisting",
             mapped.filePath,
-            path.node.loc,
+            convertBabelLoc(path.node.loc),
           );
         }
       }