parcel-bundler / parcel

The zero configuration build tool for the web. 📦🚀
https://parceljs.org
MIT License
43.48k stars 2.27k forks source link

Import single value (for security reasons) #2310

Closed doomedramen closed 4 months ago

doomedramen commented 5 years ago

My project (a node web app) has a file (config.js) sitting at its root. It contains passwords, preferences etc. I would like to pull one of these into my front end js file that is being built with parcel but when it is built the entire config.js file copied to the output.

For example:

import {apdexT} from '../../../config.js';

in my code I expect to see the val apdexT but I see the entire contents of ../../../config.js Is there a way for the rest of the fie (that is not used in the file built with parcel) to be ignored?

Thank you in advance.

doomedramen commented 5 years ago

in my built code there a like that reads:

module.exports={port:3001,db:"uptime",secret:"Change Me",showSitesToUnauth:!0,graphsOnIndex:!0,allowRegistration:!1,apdexT:.2,monitoring:{localRunner:!0,interval:5},slack:{enabled:!0,webhookURL:"..."},airdale:{enabled:!1,apiKey:""}};

This is the entire contents of the config.js file, only apdexT should have been imported.

mischnic commented 5 years ago

You could use an envfile

doomedramen commented 5 years ago

@mischnic do you have any suggestions?

mischnic commented 5 years ago

You could create a .envfile containing

API_KEY=0123456

and then use it like this: process.env.API_KEY.

(Inspired by https://github.com/parcel-bundler/parcel/issues/2299)

macklinu commented 5 years ago

I ran into a similar problem trying to import just the version from a package.json file:

package.json

{
  "name": "example",
  "description": "A description",
  "version": "1.0.0",
  "scripts": {
    "build": "parcel build index.html"
  },
  "devDependencies": {
    "parcel-bundler": "1.10.3"
  }
}

index.html

<html>
  <body>
    <script src="./index.js"></script>
  </body>
</html>

index.js

import { version } from "./package.json";

console.log(version);

When running yarn build (or npm run build) in the above example, the output dist contains a JavaScript bundle that looks like:

dist/parcel-json.ff909ce6.js

parcelRequire=function(e,r,n,t){var i="function"==typeof parcelRequire&&parcelRequire,o="function"==typeof require&&require;function u(n,t){if(!r[n]){if(!e[n]){var f="function"==typeof parcelRequire&&parcelRequire;if(!t&&f)return f(n,!0);if(i)return i(n,!0);if(o&&"string"==typeof n)return o(n);var c=new Error("Cannot find module '"+n+"'");throw c.code="MODULE_NOT_FOUND",c}p.resolve=function(r){return e[n][1][r]||r},p.cache={};var l=r[n]=new u.Module(n);e[n][0].call(l.exports,p,l,l.exports,this)}return r[n].exports;function p(e){return u(p.resolve(e))}}u.isParcelRequire=!0,u.Module=function(e){this.id=e,this.bundle=u,this.exports={}},u.modules=e,u.cache=r,u.parent=i,u.register=function(r,n){e[r]=[function(e,r){r.exports=n},{}]};for(var f=0;f<n.length;f++)u(n[f]);if(n.length){var c=u(n[n.length-1]);"object"==typeof exports&&"undefined"!=typeof module?module.exports=c:"function"==typeof define&&define.amd?define(function(){return c}):t&&(this[t]=c)}return u}({"uc/H":[function(require,module,exports) {
module.exports={name:"example",description:"A description",version:"1.0.0",scripts:{build:"parcel build index.html"},devDependencies:{"parcel-bundler":"1.10.3"}};
},{}],"Focm":[function(require,module,exports) {
"use strict";var e=require("./package.json");console.log(e.version);
},{"./package.json":"uc/H"}]},{},["Focm"], null)
//# sourceMappingURL=/parcel-json.ff909ce6.map

However, this would be the desired output bundle, since I'm only importing version.

diff --git a/dist/parcel-json.ff909ce6.js b/dist/parcel-json.ff909ce6.js
index d00e1dc..f77ef53 100644
--- a/dist/parcel-json.ff909ce6.js
+++ b/dist/parcel-json.ff909ce6.js
@@ -1,5 +1,5 @@
 parcelRequire=function(e,r,n,t){var i="function"==typeof parcelRequire&&parcelRequire,o="function"==typeof require&&require;function u(n,t){if(!r[n]){if(!e[n]){var f="function"==typeof parcelRequire&&parcelRequire;if(!t&&f)return f(n,!0);if(i)return i(n,!0);if(o&&"string"==typeof n)return o(n);var c=new Error("Cannot find module '"+n+"'");throw c.code="MODULE_NOT_FOUND",c}p.resolve=function(r){return e[n][1][r]||r},p.cache={};var l=r[n]=new u.Module(n);e[n][0].call(l.exports,p,l,l.exports,this)}return r[n].exports;function p(e){return u(p.resolve(e))}}u.isParcelRequire=!0,u.Module=function(e){this.id=e,this.bundle=u,this.exports={}},u.modules=e,u.cache=r,u.parent=i,u.register=function(r,n){e[r]=[function(e,r){r.exports=n},{}]};for(var f=0;f<n.length;f++)u(n[f]);if(n.length){var c=u(n[n.length-1]);"object"==typeof exports&&"undefined"!=typeof module?module.exports=c:"function"==typeof define&&define.amd?define(function(){return c}):t&&(this[t]=c)}return u}({"uc/H":[function(require,module,exports) {
-module.exports={name:"example",description:"A description",version:"1.0.0",scripts:{build:"parcel build index.html"},devDependencies:{"parcel-bundler":"1.10.3"}};
+module.exports={version:"1.0.0"};
 },{}],"Focm":[function(require,module,exports) {
 "use strict";var e=require("./package.json");console.log(e.version);
 },{"./package.json":"uc/H"}]},{},["Focm"], null)

Would this be a welcome addition to Parcel, and if so, could someone please help point me (or another person who would like to contribute) in the direction of where to add/edit code for this feature in the codebase? 😃

mischnic commented 5 years ago

Not sure how easy this is to do. You basically want to do tree shaking for json assets.

macklinu commented 5 years ago

My use case feels similar to this issue (tree shaking for JS assets) but not quite since my example is importing a JSON asset. Should I track JSON asset tree-shaking in a separate ticket (don't see anything open for this at the moment)?

DeMoorJasper commented 5 years ago

@macklinu JSON files get transformed to JS and JS gets transformed by babel and gathers information for treeshaking so in theory it should already do this.

Feel free to experiment with it using parcel build index.html --experimental-scope-hoisting

mischnic commented 5 years ago

so in theory it should already do this.

In practice: (with treeshaking)

(function() {
    var a = {};
    a = {
        version: "1.2.3",
        scripts: {
            build: "parcel build --target node --experimental-scope-hoisting index.js",
            "build-ts": "parcel build --target node index.js"
        },
        devDependencies: {
            "@babel/core": "^7.2.0",
            "@babel/preset-env": "^7.2.0"
        }
    };
    console.log(a.version);
})();
mischnic commented 5 years ago

If I recall correctly, parcel's scope hoisting/shaking.js visitor only removes all unreferenced variables and doesn't go into the "object" level.

Terser doesn't do it either, try it here: https://xem.github.io/terser-online/.

mischnic commented 5 years ago

Oh: terser can't optimize this

(function() {
    var a = {};
    a = {
        version: "1.2.3",
        // ...
    };
    console.log(a.version);
})();

// result:
// no difference

But only this:

(function() {
    var a = { // <---- no reassignment
        version: "1.2.3",
        // ...
    };
    console.log(a.version);
})();

// result:
console.log("1.2.3")


Parcel's scope hoisting seems to always do this:

var $ucH$exports = {};
$ucH$exports = {
    // ...
}
DeMoorJasper commented 5 years ago

@mischnic that's probably because it can cause side-effects pretty easily

mischnic commented 5 years ago

that's probably because it can cause side-effects pretty easily

But there aren't any with a JSON import, right?


Ideally, there would be no extra var $...$exports = {}; line if the asset's first access to module.exports was reassigning it (and not adding a property). (This should not have side-effects)

// ---- source
module.exports = {
  // json data
};

// ---- output
var $ucH$exports = {};
$ucH$exports = {
  // json data
};
DeMoorJasper commented 5 years ago

@mischnic not sure why this is done, thought it was side-effects, probably is. But probably on more complex scenarios than this simple example.

cc @fathyb would be great to get your input on this.

mischnic commented 5 years ago

not sure why this is done, thought it was side-effects, probably is. But probably on more complex scenarios than this simple example.

This wouldn't work with getters. But I just realised that terser doesn't run on the packager level, so this would be required after fixing the hoisting:

--- a/packages/core/parcel-bundler/src/packagers/JSConcatPackager.js
+++ b/packages/core/parcel-bundler/src/packagers/JSConcatPackager.js
@@ -542,6 +542,11 @@ class JSConcatPackager extends Packager {
       }
     }

+    output = require("terser").minify(output, { // options from transforms/terser.js
+      warnings: true,
+      safari10: true,
+    }).code;
+
     await super.write(output);
   }
fathyb commented 5 years ago

We do var exports = {}; exports = actual_exports; to be compatible with module.exports reassignements. I previously made a PR (#1939) to "optimize" this but it added too much complexity and Terser already does it so I closed it. Please see @kzc comment's: https://github.com/parcel-bundler/parcel/pull/1939#issuecomment-417188077 to fix this:

The uglify/terser unsafe compress option is actually safe for sane code - anything that does not modify builtins like polyfills. It's just being conservative by default.

Application code should specify --toplevel (or --module) to get rid of unused top level variable declarations.

You can configure Terser by adding a .terserrc file to your project.

mischnic commented 5 years ago

Well, the {compress: {unsafe: true}} options doesn't change to output. {module: true} and {toplevel: true} both result in (parameter should be "1.2.3"):

(function () {console.log({});})();
mischnic commented 5 years ago

Terser already does it so I closed it

How can terser do this already? As far as I can tell, only the assets are optimized by terser. This case can only be done by the packager

kzc commented 5 years ago

Oh: terser can't optimize this

$ cat test.js
(function() {
    var a = {};
    a = {
        version: "1.2.3",
        scripts: {
            build: "parcel build --target node --experimental-scope-hoisting index.js",
            "build-ts": "parcel build --target node index.js"
        },
        devDependencies: {
            "@babel/core": "^7.2.0",
            "@babel/preset-env": "^7.2.0"
        }
    };
    console.log(a.version, a.devDependencies["@babel/core"]);
})();

Enabling unsafe is enough for this particular example, but I generally optimize with these options:

$ cat test.js | terser -c unsafe,passes=3,pure_getters --toplevel
console.log("1.2.3","^7.2.0");

btw, Parcel uses Terser in a suboptimal way in order to improve build times. For smaller bundle size Terser should only be used once per bundle - not per JS asset with only a final mangle at the end.

You can configure Terser by adding a .terserrc file to your project.

Is this .terserrc intended for each JS asset or the final bundle? They have different goals and should probably have different configs. The issue was never resolved AFAIK.

mischnic commented 5 years ago

btw, Parcel uses Terser in a suboptimal way in order to improve build times. For smaller bundle size Terser should only be used once per bundle - not per JS asset with only a final mangle at the end.

@fathyb @devongovett This has come up several times now. Are there any reasons not to do it this way?

devongovett commented 5 years ago

Last I checked it was much faster to run terser at the asset level and only mangle the top-level scope at the end rather than run terser over the final output, which could be huge. There also wasn't a very big difference in output size that I saw, but perhaps you've run into something. Did you try running terser over the final bundle to see if it got smaller?

mischnic commented 5 years ago

Did you try running terser over the final bundle to see if it got smaller?

Mh, the only case where it helps seems to be this particular issue ("inlining a object.key lookup"). (In one project, the bundle was actually larger when removing the asset-level terser).

kzc commented 5 years ago

See: https://github.com/parcel-bundler/parcel/pull/1685#issuecomment-403388316

Given:

$ parcel -V
1.11.0
$ cat app.js
import {Sum} from './lib.js'
(new Sum(10, 4)).run();
$ cat lib.js
export class Sum {
    constructor(x, y) {
        this.x = x;
        this.y = y;
    }
    calc() {
        return this.x + this.y;
    }
    run() {
        console.log("Sum:", this.x, this.y, "=>", this.calc());
    }
}
export class Multiply {
    constructor(x, y) {
        this.x = x;
        this.y = y;
    }
    calc() {
        return this.x * this.y;
    }
    run() {
        console.log("Multiply:", this.x, this.y, "=>", this.calc());
    }
}
export class Divide {
    constructor(x, y) {
        this.x = x;
        this.y = y;
    }
    calc() {
        return this.x / this.y;
    }
    run() {
        console.log("Divide:", this.x, this.y, "=>", this.calc());
    }
}
export class Minus {
    constructor(x, y) {
        this.x = x;
        this.y = y;
    }
    calc() {
        return this.x - this.y;
    }
    run() {
        console.log("Minus:", this.x, this.y, "=>", this.calc());
    }
}

Per-asset minification enabled yields a 1216 byte bundle:

$ parcel build ./app.js --experimental-scope-hoisting
✨  Built in 448ms.

dist/app.js    1.19 KB    353ms

Per-asset minification disabled - this unminified bundle size is not important:

$ parcel build ./app.js --no-minify --experimental-scope-hoisting
✨  Built in 362ms.

dist/app.js    2.51 KB    268ms

Running that unminified bundle through terser mangle/compress yields a 564 byte bundle:

$ terser dist/app.js -mc | wc -c
     564
mischnic commented 5 years ago

Some notes:

mischnic commented 5 years ago

I've created a POC for testing: yarn add @mischnic/parcel-bundler. Changes: https://github.com/parcel-bundler/parcel/compare/experiment-scopehoist-terser

mischnic commented 5 years ago

Rollup (REPL) handles this just fine (and the POC above as well), but I don't think they're using terser?

import { cube } from './maths.js';

if(cube)
    console.log("YES");
else
    console.log("NO");

// maths.js
export const cube = false;
export const square = false;
doomedramen commented 5 years ago

Thank you for the heads up about how Rollup handles this @mischnic

mischnic commented 5 years ago

@kzc What made the non-global terser invocation perform bad in this situations has been that Parcel didn't know about the __PURE__ comments, with the babel classes (your example) and when added manually (#2646). (Though it wouldn't solve this specific issue) (I'm still not saying that the per-asset parser is the best way regarding code size).

As an alternative to running terser on the whole bundle, I made the scope hoisting algorithm be aware of these PURE comments (at its core a 3 line change). The only issue I've run into is that terser doesn't keep the PURE comments when we run it on the asset, cli equivalent:

$ echo "var a = /*@__PURE__*/foobar();" | terser -c --comments "/__PURE__/"
var a=/* */foobar();
$ echo "var a = /*@__PURE__*/foobar();" | terser -c --comments "all"
var a=/* */foobar();

Can this be achieved with terser, ~is this a bug or~ I'm afraid it's intended?

WIP branch: https://github.com/parcel-bundler/parcel/compare/treeshake-pure

kzc commented 5 years ago

I no longer work on terser. The pure annotation removal was intentionally introduced in uglify-js (and inherited by terser) to prevent the comments incorrectly shifting to non pure code after the compress pass and producing incorrect results. It was the simplest solution at the time to avoid some rather tricky corner cases.

Rollup has also recently implemented uglify-js compatible pure annotations and incorporated the corresponding uglify-js tests. This PR discusses the issues involved in its implementation: https://github.com/rollup/rollup/pull/2429

mischnic commented 5 years ago

Thank you for your quick response

It was the simplest solution at the time to avoid some rather tricky corner cases.

Makes sense.

Rollup has also recently implemented uglify-js compatible pure annotations and incorporated the corresponding uglify-js tests.

My first idea was: 1) run terser on asset level 2) tree shake more intelligently using the pure comments to prevent running terser on the whole output. The first part is impossible (because the pure comments get removed) and the second requires quite a bit of effort (I'm not sure if this more difficult to do with parcel's current logic compared to Rollup). and while the pure comment handling seems doable, it wouldn't add much value because we can't run terser beforehand (asset) and don't want to run terser afterwards (bundle). If we want to run terser and have comparable bundle sizes, we'll have to run terser on the output... @devongovett Another perspective: the pure handling could help tree shake more code, so that terser has to process less code on the bundle level

kzc commented 3 years ago

@mischnic I don't know where this issue stands and perhaps you guys found another solution with terser in the past couple of years, but I thought I'd pass on that uglify-js just implemented the ability to emit pure annotations in minified output: https://github.com/mishoo/UglifyJS/pull/4763. This annotations feature has been committed but not yet released. uglify-js now supports ES6+ up to ES2020. It's a different code base from terser and each have some optimizations that the other may lack.

mischnic commented 3 years ago

Thanks. Parcel 2 already runs Terser at the end (so on the bundled output).

Should be pretty easy to write an optimizer plugin that uses uglify-js instead, might be interesting to compare the two.

kzc commented 3 years ago

@mischnic Cool. Please share your findings and open an issue with uglify-js if there's a problem.

devongovett commented 4 months ago

This is possible in a number of ways:

  1. Rely on tree shaking
  2. Use an environment variable
  3. Use macros