Babel helper things #358

emmatown · 2021-01-05T00:12:23Z

Closes #318

cc @Andarist thought you might be interested in this

changeset-bot · 2021-01-05T00:12:25Z

🦋 Changeset detected

Latest commit: eebf097

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@preconstruct/cli	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

codecov · 2021-01-05T00:14:06Z

Codecov Report

Merging #358 (c503b63) into master (b9e9137) will increase coverage by 0.22%.
The diff coverage is 97.05%.

@@            Coverage Diff             @@
##           master     #358      +/-   ##
==========================================
+ Coverage   89.34%   89.56%   +0.22%     
==========================================
  Files          32       32              
  Lines        1276     1303      +27     
  Branches      322      336      +14     
==========================================
+ Hits         1140     1167      +27     
  Misses        132      132              
  Partials        4        4

Impacted Files	Coverage Δ
packages/cli/src/worker.ts	`91.30% <ø> (ø)`
packages/cli/src/rollup-plugins/babel.ts	`98.43% <95.45%> (-1.57%)`	⬇️
packages/cli/src/build/config.ts	`84.21% <100.00%> (+2.72%)`	⬆️
packages/cli/src/build/rollup.ts	`90.47% <100.00%> (+0.82%)`	⬆️
.../cli/src/rollup-plugins/flow-and-prod-dev-entry.ts	`87.75% <100.00%> (+0.25%)`	⬆️
...rc/rollup-plugins/rewrite-babel-runtime-helpers.ts	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9e9137...eebf097. Read the comment docs.

Andarist · 2021-01-05T10:17:30Z

packages/cli/src/build/config.ts

@@ -57,6 +57,11 @@ function getGlobal(project: Project, name: string) {
  }
 }

+const babelHelperId = /@babel\/runtime(|-corejs[23])\/helpers\//;
+
+const interop = (id: string | null): "auto" | "default" =>


TIL that this can be a function

Have you checked how this will behave in Babel 8? I know that more than this would have to be re-checked to check against Babel 8 compatibility but maybe worth to be future-proof here.

In Babel 8 there won't be separate cjs/esm versions of helpers as they have started to use conditional exports. You would have to check if interop: "default" works with Babel 8 though - there might have been changes around this (but they might have also left it as is - I don't recall, I think Nicolo has wanted to support .default-less requires in node so probably this has not been touched but... well, worth re-checking).

Given the PR about it isn't merged yet, I'm not overly concerned about it. Preconstruct has a direct dependency on @babel/core@7 rn anyway.

Andarist · 2021-01-05T10:22:43Z

packages/cli/src/build/__tests__/basic.ts

+    ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ dist/test.cjs.dev.js, dist/test.cjs.prod.js ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
+    'use strict';
+
+    var _createForOfIteratorHelper = require('@babel/runtime/helpers/createForOfIteratorHelper');


shouldnt u add a very similar test that would check that this helper has been inlined rather than required?

As in when the version of @babel/runtime doesn't include that helper? That test already exists before this PR -

preconstruct/packages/cli/src/build/__tests__/basic.ts

Lines 238 to 367 in b9e9137

test("does not duplicate babel helpers when using @babel/plugin-transform-runtime but the helper isn't in the version of @babel/runtime that the user has specified", async () => {

const dir = await testdir({

"package.json": JSON.stringify({

name: "test",

main: "dist/test.cjs.js",

dependencies: {

"@babel/runtime": "*",

},

}),

"babel.config.json": JSON.stringify({

presets: [require.resolve("@babel/preset-env")],

plugins: [require.resolve("@babel/plugin-transform-runtime")],

}),

"src/index.js": "import './other'; for (const x of something) {}",

"src/other.js": "for (const x of something) {}",

});

await build(dir);

expect(await getDist(dir)).toMatchInlineSnapshot(`

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ dist/test.cjs.dev.js, dist/test.cjs.prod.js ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

'use strict';

function _arrayLikeToArray(arr, len) {

if (len == null || len > arr.length) len = arr.length;

for (var i = 0, arr2 = new Array(len); i < len; i++) arr2[i] = arr[i];

return arr2;

}

function _unsupportedIterableToArray(o, minLen) {

if (!o) return;

if (typeof o === "string") return _arrayLikeToArray(o, minLen);

var n = Object.prototype.toString.call(o).slice(8, -1);

if (n === "Object" && o.constructor) n = o.constructor.name;

if (n === "Map" || n === "Set") return Array.from(n);

if (n === "Arguments" || /^(?:Ui|I)nt(?:8|16|32)(?:Clamped)?Array$/.test(n)) return _arrayLikeToArray(o, minLen);

}

function _createForOfIteratorHelper(o) {

if (typeof Symbol === "undefined" || o[Symbol.iterator] == null) {

if (Array.isArray(o) || (o = _unsupportedIterableToArray(o))) {

var i = 0;

var F = function () {};

return {

s: F,

n: function () {

if (i >= o.length) return {

done: true

};

return {

done: false,

value: o[i++]

};

},

e: function (e) {

throw e;

},

f: F

};

}

throw new TypeError("Invalid attempt to iterate non-iterable instance.\\nIn order to be iterable, non-array objects must have a [Symbol.iterator]() method.");

}

var it,

normalCompletion = true,

didErr = false,

err;

return {

s: function () {

it = o[Symbol.iterator]();

},

n: function () {

var step = it.next();

normalCompletion = step.done;

return step;

},

e: function (e) {

didErr = true;

err = e;

},

f: function () {

try {

if (!normalCompletion && it.return != null) it.return();

} finally {

if (didErr) throw err;

}

}

};

}

var _iterator = _createForOfIteratorHelper(something),

_step;

try {

for (_iterator.s(); !(_step = _iterator.n()).done;) {

var x = _step.value;

}

} catch (err) {

_iterator.e(err);

} finally {

_iterator.f();

}

var _iterator$1 = _createForOfIteratorHelper(something),

_step$1;

try {

for (_iterator$1.s(); !(_step$1 = _iterator$1.n()).done;) {

var x$1 = _step$1.value;

}

} catch (err) {

_iterator$1.e(err);

} finally {

_iterator$1.f();

}

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ dist/test.cjs.js ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

'use strict';

if (process.env.NODE_ENV === "production") {

module.exports = require("./test.cjs.prod.js");

} else {

module.exports = require("./test.cjs.dev.js");

}

`);

});

a similar test - yes, but without @babel/plugin-transform-runtime being used

Andarist · 2021-01-05T10:30:13Z

packages/cli/src/rollup-plugins/flow-and-prod-dev-entry.ts

@@ -49,6 +49,7 @@ export default function flowAndNodeDevProdEntry(
      }

      if (
+        source.startsWith("\0") ||


wait - is \0babelWhatever added to the top of a generated virtual source code? 🤔 how does it work? kinda blurry for me in a rush

source is referring to the unresolved module specifier, not source code.

Oh, gotcha.

Andarist · 2021-01-05T10:31:06Z

How do u ensure that this doesn't clash with configurations using the transform plugin explicitly? 🤔

emmatown · 2021-01-05T12:31:32Z

How do u ensure that this doesn't clash with configurations using the transform plugin explicitly? 🤔

preconstruct/packages/cli/src/worker.ts

Lines 9 to 14 in 2f63b87

    
           const previousHelperGenerator = file.get("helperGenerator"); 
        
           file.set("helperGenerator", (name: string) => { 
        
             if (previousHelperGenerator) { 
        
               const helperFromPrev = previousHelperGenerator(name); 
        
               if (helperFromPrev != null) return helperFromPrev; 
        
             }

(that plugin is always last)

Andarist

From whatever my ✅ is worth here:

✅ ✅ ✅ ✅ ...

Great attention to little things as always 👍

emmatown added 2 commits January 5, 2021 09:23

Babel helper things

69b9f64

Add a changeset

2f63b87

Andarist reviewed Jan 5, 2021

View reviewed changes

another test

eebf097

Andarist approved these changes Jan 5, 2021

View reviewed changes

emmatown merged commit e3b4196 into master Jan 5, 2021

emmatown deleted the babel-helper-things branch January 5, 2021 20:20

github-actions bot mentioned this pull request Jan 5, 2021

Version Packages #359

Closed

github-actions bot mentioned this pull request Jan 22, 2021

Version Packages #363

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Babel helper things #358

Babel helper things #358

emmatown commented Jan 5, 2021

changeset-bot bot commented Jan 5, 2021 •

edited

codecov bot commented Jan 5, 2021 •

edited

Andarist Jan 5, 2021

Andarist Jan 5, 2021

emmatown Jan 5, 2021

Andarist Jan 5, 2021

emmatown Jan 5, 2021

Andarist Jan 5, 2021

Andarist Jan 5, 2021

emmatown Jan 5, 2021

Andarist Jan 5, 2021

Andarist commented Jan 5, 2021

emmatown commented Jan 5, 2021

Andarist left a comment

	test("does not duplicate babel helpers when using @babel/plugin-transform-runtime but the helper isn't in the version of @babel/runtime that the user has specified", async () => {
	const dir = await testdir({
	"package.json": JSON.stringify({
	name: "test",
	main: "dist/test.cjs.js",
	dependencies: {
	"@babel/runtime": "*",
	},
	}),
	"babel.config.json": JSON.stringify({
	presets: [require.resolve("@babel/preset-env")],
	plugins: [require.resolve("@babel/plugin-transform-runtime")],
	}),
	"src/index.js": "import './other'; for (const x of something) {}",
	"src/other.js": "for (const x of something) {}",
	});
	await build(dir);
	expect(await getDist(dir)).toMatchInlineSnapshot(`
	⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ dist/test.cjs.dev.js, dist/test.cjs.prod.js ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
	'use strict';

	function _arrayLikeToArray(arr, len) {
	if (len == null \|\| len > arr.length) len = arr.length;

	for (var i = 0, arr2 = new Array(len); i < len; i++) arr2[i] = arr[i];

	return arr2;
	}

	function _unsupportedIterableToArray(o, minLen) {
	if (!o) return;
	if (typeof o === "string") return _arrayLikeToArray(o, minLen);
	var n = Object.prototype.toString.call(o).slice(8, -1);
	if (n === "Object" && o.constructor) n = o.constructor.name;
	if (n === "Map" \|\| n === "Set") return Array.from(n);
	if (n === "Arguments" \|\| /^(?:Ui\|I)nt(?:8\|16\|32)(?:Clamped)?Array$/.test(n)) return _arrayLikeToArray(o, minLen);
	}

	function _createForOfIteratorHelper(o) {
	if (typeof Symbol === "undefined" \|\| o[Symbol.iterator] == null) {
	if (Array.isArray(o) \|\| (o = _unsupportedIterableToArray(o))) {
	var i = 0;

	var F = function () {};

	return {
	s: F,
	n: function () {
	if (i >= o.length) return {
	done: true
	};
	return {
	done: false,
	value: o[i++]
	};
	},
	e: function (e) {
	throw e;
	},
	f: F
	};
	}

	throw new TypeError("Invalid attempt to iterate non-iterable instance.\\nIn order to be iterable, non-array objects must have a [Symbol.iterator]() method.");
	}

	var it,
	normalCompletion = true,
	didErr = false,
	err;
	return {
	s: function () {
	it = o[Symbol.iterator]();
	},
	n: function () {
	var step = it.next();
	normalCompletion = step.done;
	return step;
	},
	e: function (e) {
	didErr = true;
	err = e;
	},
	f: function () {
	try {
	if (!normalCompletion && it.return != null) it.return();
	} finally {
	if (didErr) throw err;
	}
	}
	};
	}

	var _iterator = _createForOfIteratorHelper(something),
	_step;

	try {
	for (_iterator.s(); !(_step = _iterator.n()).done;) {
	var x = _step.value;
	}
	} catch (err) {
	_iterator.e(err);
	} finally {
	_iterator.f();
	}

	var _iterator$1 = _createForOfIteratorHelper(something),
	_step$1;

	try {
	for (_iterator$1.s(); !(_step$1 = _iterator$1.n()).done;) {
	var x$1 = _step$1.value;
	}
	} catch (err) {
	_iterator$1.e(err);
	} finally {
	_iterator$1.f();
	}

	⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ dist/test.cjs.js ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
	'use strict';

	if (process.env.NODE_ENV === "production") {
	module.exports = require("./test.cjs.prod.js");
	} else {
	module.exports = require("./test.cjs.dev.js");
	}

	`);
	});

Babel helper things #358

Babel helper things #358

Conversation

emmatown commented Jan 5, 2021

changeset-bot bot commented Jan 5, 2021 • edited

🦋 Changeset detected

codecov bot commented Jan 5, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Andarist commented Jan 5, 2021

emmatown commented Jan 5, 2021

Andarist left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Jan 5, 2021 •

edited

codecov bot commented Jan 5, 2021 •

edited