Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polyfills/dependencies "magically" included by mentioning in comments or strings #1569

Closed
is-already-taken opened this issue Jun 17, 2018 · 3 comments

Comments

@is-already-taken
Copy link

🐛 bug report

Parcel will include polyfills/dependencies if classnames or keywords are mentioned in a comment or string.

🎛 Configuration (.babelrc, package.json, cli command)

No configuration, ran with:

$  node_modules/.bin/parcel build index.js

🤔 Expected Behavior

Parcel shall either

  • include proper polyfills when needed by fs.readFileSync() or
  • leave the missing dependency unresolved

😯 Current Behavior

Parcel includes dependencies when mentioning global (class) names in a comment or string.

🔦 Context

According to question #1370 inlining CSS into Javascript is possible by using fs.readFileSync().
That example did not work out of box. While trying to fix this with a workaround:

const Buffer = atob;

... I step by step discovered that the word Buffer includes a lot of polyfill code if the line was commented out.

💻 Code Sample

index.js (causing a Uncaught ReferenceError: Buffer is not defined exception)

import fs from 'fs';

(function() {
	var styles = fs.readFileSync('./index.css');

	console.log(styles);

	var head = document.getElementsByTagName("head")[0],
		style;

	style = document.createElement("style");
	style.setAttribute("type", "text/css");
	style.innerText = styles;

	head.appendChild(style);
})();

index.js (causing no exception)

import fs from 'fs';

// Magically include missing dependency "Buffer" by merely mentioning the word. :O

(function() {
	var styles = fs.readFileSync('./index.css');

	console.log(styles);

	var head = document.getElementsByTagName("head")[0],
		style;

	style = document.createElement("style");
	style.setAttribute("type", "text/css");
	style.innerText = styles;

	head.appendChild(style);
})();

test.html

<!DOCTYPE html>
<html>
<head>
	<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
</head>
<body>
	<script src="dist/index.js" type="text/javascript"></script>
</body>
</html>

🌍 Your Environment

Software Version(s)
Parcel 1.9.1
Node 8.11.1
npm/Yarn 5.6.0
Operating System Linux
@devongovett
Copy link
Member

The fs.readFileSync call requires Buffer by default, but if you pass an encoding as the second argument it will return a string instead.

fs.readFileSync('filename', 'utf8')

@is-already-taken
Copy link
Author

Thanks. But that does not explain why there is a lot of code included if I write the word "Buffer" in a comment.

Without the comment:

-rw-r--r-- 1 dev dev 1441 Jun 18 07:00 dist/index.js

With the comment:

-rw-r--r-- 1 dev dev 24032 Jun 18 07:01 dist/index.js

This is included when the comment with the word "Buffer" is present:

},{}],6:[function(require,module,exports) {
"use strict";exports.byteLength=u,exports.toByteArray=i,exports.fromByteArray=d;for(var r=[],t=[],e="undefined"!
...
var global = arguments[3];
var t=arguments[3],r=require("base64-js"),e=require("ieee754"),n=require("isarray");function i(){try{var t=new Uint8Array(1);return t.__proto__={__proto__:Uint8Array.prototype,foo:function(){return 42}},42===t.foo()&&"functi
...

At least 42 is present. phew

@TerrorJack
Copy link

@devongovett It would be nice to document the fs.readFileSync inlining behavior somewhere in docs, at least the part of not working with binary files & browser targets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants