Use ES6 for contrib and custom JavaScript development

+1 for the idea! Enabling contrib people to use es6 is really nice! One thing to keep in mind though is that they cannot add external packages easily yet, but I think that's fine.

I'm curious whether instead of adding --contrib/--dev or anything like that the simpler option would be a --dir option.

I was curious whether it is possible to see this problem in the diff.

1. +++ b/core/misc/tabledrag.js
   @@ -4,11 +4,11 @@
   -var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol" ? function (obj) { return typeof obj; } : function (obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj; };
   +
   +function _typeof(obj) { if (typeof Symbol === "function" && _typeof(Symbol.iterator) === "symbol") { _typeof = function (_typeof2) { function _typeof(_x) { return _typeof2.apply(this, arguments); } _typeof.toString = function () { return _typeof2.toString(); }; return _typeof; }(function (obj) { return typeof obj === "undefined" ? "undefined" : _typeof(obj); }); } else { _typeof = function (_typeof3) { function _typeof(_x2) { return _typeof3.apply(this, arguments); } _typeof.toString = function () { return _typeof3.toString(); }; return _typeof; }(function (obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj === "undefined" ? "undefined" : _typeof(obj); }); } return _typeof(obj); }
   

   This is one of the differences which could make a difference.

2. +++ b/core/misc/tabledrag.js
   @@ -546,21 +537,21 @@ var _typeof = typeof Symbol === "function" && typeof Symbol.iterator === "symbol
          if (y > rowY - rowHeight && y < rowY + rowHeight) {
   -        if (_this3.indentEnabled) {
   -          if (Object.keys(_this3.rowObject.group).some(function (o) {
   +        if (this.indentEnabled) {
   +          if (Object.keys(this.rowObject.group).some(function (o) {
                return _this3.rowObject.group[o] === row;
              })) {
   ...
   -        } else if (row === _this3.rowObject.element) {
   +        } else if (row === this.rowObject.element) {
                return {
                  v: null
   ...
    
   -        if (!_this3.rowObject.isValidSwap(row)) {
   +        if (!this.rowObject.isValidSwap(row)) {
   

   Maybe this makes a negative difference too.

Do you want to try out this locally? Otherwise it might be worth reporting the problem.

I am getting the following error with this patch.

$ yarn run watch:js --dir ../
yarn run v1.5.1
$ node ./scripts/js/babel-es6-watch.js --dir ../
/var/www/d8/docroot/core/node_modules/snapdragon/lib/parser.js:473
        throw new Error('no parsers registered for: "' + self.input.slice(0, 5) + '"');
        ^

Error: no parsers registered for: "]a(r)"
    at parse (/var/www/d8/docroot/core/node_modules/snapdragon/lib/parser.js:473:15)
    at Parser.parse (/var/www/d8/docroot/core/node_modules/snapdragon/lib/parser.js:477:24)
    at Snapdragon.parse (/var/www/d8/docroot/core/node_modules/snapdragon/index.js:122:28)
    at Snapdragon.<anonymous> (/var/www/d8/docroot/core/node_modules/braces/lib/braces.js:37:45)
    at Braces.parse (/var/www/d8/docroot/core/node_modules/braces/lib/braces.js:74:26)
    at Braces.expand (/var/www/d8/docroot/core/node_modules/braces/lib/braces.js:96:18)
    at create (/var/www/d8/docroot/core/node_modules/braces/index.js:142:15)
    at memoize (/var/www/d8/docroot/core/node_modules/braces/index.js:293:13)
    at Function.braces.create (/var/www/d8/docroot/core/node_modules/braces/index.js:161:10)
    at Function.braces.expand (/var/www/d8/docroot/core/node_modules/braces/index.js:82:17)
error An unexpected error occurred: "Command failed.
Exit code: 1
Command: sh
Arguments: -c node ./scripts/js/babel-es6-watch.js --dir ../
Directory: /var/www/d8/docroot/core
Output:
".
info If you think this is a bug, please open a bug report with the information provided in "/var/www/d8/docroot/core/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

The good news is that yarn run watch:js --dir ../modules works well. So the problem lays somewhere in core JS files.

The good news is that yarn run watch:js --dir ../modules works well.

Well, it does not. The only change in processed file is "DO NOT EDIT THIS FILE." header. I wounder if Babel presets are picked correctly when the root directory is changed.

https://github.com/babel/babel/releases/tag/v7.0.0-rc.0 is out

babel 7.0 was released a month ago. Can we here proceed now?

Am I missing something in this PR? Why are we doing two things at a time:

• Updating to babel 7.x
• Enable usage for contrib and custom modules

I think focusing on just the later one would be a more successful strategy.

What exactly is this postponed on? On upstream features in babel? That was the state in #9 more than 14 months ago. Is that still true?

✔

Use `cwd` I'd say.

Furthermore,
If we need more flexible, I will export the whole chokidar options (, or the Drupal script options) via Cli or config.js or both.

things like this:
https://github.com/prettier/prettier/blob/master/src/cli/minimist.js

(There're some projects structures in Drupal is a bit odd, I believe config.js, a programmable config file is more suitable for Drupal. )

I'm not sure this is the correct approach. From my perspective, this patch attempts to do too many things at once, making it quite difficult to review. If you wanted to the easiest path, excluding some limitations, to using core standards to transpile contrib JavaScript than the work in #3047534: Allow core transpile tooling to work with files outside the default directory. would have solved that problem. I'm still not sure I understand this need though.

However we've now perhaps resolved the original goal here, updated babel, and adjusted the output of all core JavaScript files. I would really suggest to break this up into a few smaller issues to make it more digestible for reviewers and committers.

Thanks.

I don't really understand why core should contain code for doing ES6 to ES5 compilation to contrib. Usually, each project should be responsible for their own tooling. I'm wondering if a better approach would be to create a separate library for our compilation process that contrib projects could depend on if they want to.

I don't really understand why core should contain code for doing ES6 to ES5 compilation to contrib.

For me it would be convenient and an easy way to ensure the JavaScript I write is compliant with the Drupal standards. I must admit I had issues to even get started.

https://www.drupal.org/docs/develop/standards/javascript/javascript-codi...

States that the reasonable coding standard is 'Airbnb JavaScript Style Guide'
And then there is a separate code standard list below.
And then if we look at the Airbnb JavaScript Style Guide we can see that ES5 is deprecated which means I should probably use ES6.

So if we decide, okay, let's go ES6

https://eslint.org/docs/user-guide/command-line-interface
https://www.drupal.org/node/1955232

These configuration files ship with Drupal 8 core and can be used from there.

.eslintrc.json
.eslintignore
In a Drupal 8 folder these configuration files will be automatically detected and used by ESLint when it is invoked from within the code base.

Following those instructions, which leads us to another problem: https://www.drupal.org/project/drupal/issues/3003522 where the suggestion is to set your own .eslintrc as root, which according to article above I shouldn't need to etc.

With that being said, all of the above could be solved by just improving the documentation and slightly and we're fine, but then again, if the contrib. module just needs some JavaScript for a change, or enhancement I'm not that keen to setup my own tooling. I just want something that works. For greater projects where JavaScript is a key of the module, sure, your own tooling makes more sense.

I'm wondering if a better approach would be to create a separate library for our compilation process that contrib projects could depend on if they want to.

Yeah, this approach would also solve the DX-issues I had above.

At some point we might want to acknowledge the fact that JavaScript needs preprocessing.

I disagree. Browsers have made a big break in supporting ES6 and Drupal Core JavaScripts are mostly quiet simple. They are using ES6 features that are supported by all modern browsers.
https://caniuse.com/#search=es6
Once we drop support of Internet Explorer there will probably be no point to use Babel for compiling JavaScript.

Same applies to CSS. Custom properties make SASS less essential.

Preprocessor is still worth to introduce to Core. Packages like preset-env allow us to forget what we're writing for. However, for package bundlers, I think we need a proven before adding into Core. e.g.: Is it possible and how well the code splitting..etc. Personally, I don't like a mix of PHP & NODEJS code to get a simple thing work.

Second, I think when Drupal introduce these ES6 scripts into CORE, we (I) intended to make it CORE (@internal) only. At that time, I hope @internal allow the CORE JS development to move forward faster. I'm tried to see the word "BC, BC, BC". Therefore, I suggested in #26 for global configs. If we opening a hole, make everything configurable. Each option has a reason for a new patch.

In #26, cwd is a chokidar option. We need not create another --dir and write a doc to explain how it works. Or going to figure out the difference between these options.

#2658650: [META] Optimize Frontend Workflow in Core Development

@lauriii Very, very few contrib modules will ever figure this out. If we want contrib to follow contrib's lead/example, then we should make it feasible to do so.

I think @johndevman's comment is accurate in describing how tricky it is to follow core's example. I also struggled with it while working on https://www.drupal.org/project/entity_embed a few months ago.

@Wim Leers - I think the thing to understand is that core does things if a non-optimal way. There are original design constraints that force us to do the things the way we are currently. Contrib shouldn't feel constrained by these decisions.

Sure. But even then it's up to core to provide guidance to contrib. If it can encourage contrib to do better than itself: wonderful! 😀🥳

I think we agree that contrib is lacking proper tooling for ES6 to ES5 transpilation process. The easiest way forward would probably be creating a library that could be loaded through npm which is built without Drupal core limitations. We could also try to build proper tooling in core but that will likely take much longer. There's already some related discussion happening in #2873160: Implement core management of 3rd-party FE libraries.

I think, for now, we should say that the core ES6 compilation process is not supported beside what is needed for core since the workflow is suboptimal. As a result, we shouldn't fix bugs in the transpilation process, unless it affects core.

@johndevman's comment brings out many hidden issues:

- following the CORE way to develop
- following the CORE code standard
- Disconnect from CORE
- Enable CLI to lint codes.
- Enable editor to lint codes.
- Combine core & custom setup
-- etc..

To move the package.json one level up -- The only way to get these things done perfectly. (but not the NODEJS world preferred?)

For linting, the basic step you could do now without patch:

cd /core
yarn install --modules-folder ../core

To copy devDependencies doesn't work I think, now this doesn't specify the exact version for some packages. eg. "^17.0.0".

Or
(if you need your own package.json)

cp yarn.lock modules/custom/yarn.lock
cp package.json modules/custom/package.json
cd modules/custom
....... edit package.json.......
yarn

there's an installation problem on node side, you can't do it very straightly.
https://www.npmjs.com/package/eslint-config-airbnb

FWIW, it is currently possible to use core tooling to build (not watch) custom ES6 files. For example:

# in core/> yarn run build:js --file ../modules/custom/**/*.es6.js

However, I think there is some problem with the output (it is not really transpiled, just reformatted).

Closing as outdated now that the build step is no longer in use - contrib can just directly write ES6 JavaScript as soon as they drop compatibility with IE11.

#3278415: Remove usages of the JavaScript ES6 build step, the build step itself, and associated dev dependencies