Problem/Motivation

- Increasing the Performance on compile files
- Worked in Windows

Proposed resolution

- Simplify fork `babel-es6-watch.js` and modify it.

CommentFileSizeAuthor
#18 core-es6-babel-compile-2818409-18.patch3.49 KBnod_
#15 interdiff.patch374 bytesdroplet
#15 core-es6-babel-compile-2818409-15.patch3.41 KBdroplet
#9 interdiff.patch1.73 KBdroplet
#9 core-es6-babel-compile-2818409-8.patch3.38 KBdroplet
#5 interdiff.txt863 bytesnod_
#5 core-es6-babel-compile-2818409-5.patch2.73 KBnod_
#3 core-es6-babel-compile-2818409-3.patch11.15 KBnod_
#3 interdiff.txt1.79 KBnod_
babel-es-6-compile.patch2.64 KBdroplet

Comments

droplet created an issue. See original summary.

droplet’s picture

droplet commented 
+++ b/core/scripts/js/babel-es6-compile.js
@@ -0,0 +1,59 @@
+    process.exit(0);

It doesn't EXIT safe. Needs a thought how to keep it simple.

nod_’s picture

nod_
Primary language French
Location Lille
commented
StatusFileSize
new interdiff.txt1.79 KB
new core-es6-babel-compile-2818409-3.patch11.15 KB

used glob to find all the files, like eslint does.

droplet’s picture

droplet commented

Just don't want to add another package but looking into node_modules. glob has already there. So it's fine.

+++ b/core/scripts/js/babel-es6-compile.js
@@ -45,15 +39,18 @@
+    fs.writeFile(mapLoc, JSON.stringify(result.map));
...
+    fs.writeFile(`${fileName}.js`, result.code);
...
     log(`'${filePath}' has been compiled.`);

async, we can simply tell the file is being processed instead

nod_’s picture

nod_
Primary language French
Location Lille
commented
StatusFileSize
new core-es6-babel-compile-2818409-5.patch2.73 KB
new interdiff.txt863 bytes
droplet’s picture

droplet commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/package.json
@@ -4,13 +4,14 @@
+    "glob": "^7.1.1"

this package won't affected code gen, no locked version is fine.

+++ b/core/scripts/js/babel-es6-compile.js
@@ -0,0 +1,54 @@
+const globOptions = { ignore: 'node_modules/**' };

little space problem which able to fix when it's commit

Thanks nod_!

droplet’s picture

droplet commented
Status: Reviewed & tested by the community » Needs work

wrong sourcemap, all sourcemaps have same name :s
{"version":3,"sources":["nav-tabs.es6.js"]

droplet’s picture

droplet commented 
+++ b/core/scripts/js/babel-es6-compile.js
@@ -0,0 +1,54 @@
+  babel.transformFile(filePath, babelOptions, function (err, result) {

this line has problem. use transformFileSync works but I don't understand why.. :S

droplet’s picture

droplet commented
Status: Needs work » Needs review
StatusFileSize
new core-es6-babel-compile-2818409-8.patch3.38 KB
new interdiff.patch1.73 KB

have a look into Babel source. kind of Closure issue. I think it's a bug in Babel. Never expected it works that way.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community

yup, that works now, nice catch. I guess not mutating function parameters is important :)

nod_’s picture

nod_
Primary language French
Location Lille
commented

Tested on a windows box. It works.

star-szr’s picture

star-szr
he/him
Primary language English
commented

Seems like a very clear win. I tested by doing (from core):

sh scripts/js/rename-js-files-to-es6.sh
npm run-script build:js

Ran a lot faster than the previous script and with lots more feedback. The older way also didn't exit out after finishing for me.

For me, the older script also didn't generate the names in the source map whereas the node one does.

+++ b/core/package.json
@@ -4,13 +4,14 @@
     "babel-cli": "6.16.0",
     "babel-preset-es2015": "6.16.0",
-    "chokidar": "1.6.0"
+    "chokidar": "1.6.0",
+    "glob": "^7.1.1"

Should this new dependency have a specific version string like the others?

droplet’s picture

droplet commented

with ^, there's no APIs breaking changes usually. My thought is to let it up-to-date and fix any system/nodejs problem.

(Note: we won't lock the NODE version)

droplet’s picture

droplet commented
Assigned: Unassigned » droplet
Status: Reviewed & tested by the community » Needs work
droplet’s picture

droplet commented
Assigned: droplet » Unassigned
Status: Needs work » Needs review
StatusFileSize
new core-es6-babel-compile-2818409-15.patch3.41 KB
new interdiff.patch374 bytes

We don't need babel-cli anymore.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Needs a reroll.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs work » Needs review
StatusFileSize
new core-es6-babel-compile-2818409-18.patch3.49 KB

Renamed babel-es6-compile.js to babel-es6-build.js to follow the name of the npm script.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Reviewed & tested by the community

Thanks that name was also hard to spot in the patch because .sh turned to .js at first glance.

Tested via commands in #12

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 87cf922 and pushed to 8.3.x. Thanks!

  • alexpott committed 87cf922 on 8.3.x 
    Issue #2818409 by droplet, nod_: Convert Babel ES6 Compile script to...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.