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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

droplet created an issue. See original summary.

droplet’s picture

droplet CreditAttribution: 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
CreditAttribution: nod_ as a volunteer commented
FileSize
interdiff.txt1.79 KB
core-es6-babel-compile-2818409-3.patch11.15 KB

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

droplet’s picture

droplet CreditAttribution: 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
CreditAttribution: nod_ as a volunteer commented
FileSize
core-es6-babel-compile-2818409-5.patch2.73 KB
interdiff.txt863 bytes
droplet’s picture

droplet CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: 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 CreditAttribution: droplet commented
Status: Needs work » Needs review
FileSize
core-es6-babel-compile-2818409-8.patch3.38 KB
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
CreditAttribution: nod_ as a volunteer 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
CreditAttribution: nod_ as a volunteer commented

Tested on a windows box. It works.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr as a volunteer 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 CreditAttribution: 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 CreditAttribution: droplet commented
Assigned: Unassigned » droplet
Status: Reviewed & tested by the community » Needs work
droplet’s picture

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

We don't need babel-cli anymore.

nod_’s picture

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

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

Needs a reroll.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer commented
Status: Needs work » Needs review
FileSize
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
Primary language English
Location Vancouver
CreditAttribution: joelpittet 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 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three 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.