Ckeditor5: The async keyword should be followed by a single space

[ma2ciek]

The correct code for the async arrow function with the current eslint-config-ckeditor5 rules looks like this:

it( 'should display header message', async() => {} );

While the proper code style IMO should look like this:

it( 'should display header message', async () => {} );

Unfortunately, it's a BC for anybody using our preset and the async/await syntax.

cc @jodator @Reinmar

Ref https://eslint.org/docs/rules/space-before-function-paren.

[jodator]

I'm fine with breaking change if it will make code style OK.

Also should be tested for all variants:

async () => {}
async function() {}
// etc

[Reinmar]

If the change will apply to async arrow functions only, then I'm ok as well.

[jodator]

I was checking the PR and I have a question: Where is this used? (sorry for not checking it earlier).

Right now we have a definition:

module.exports = {
    extends: 'eslint:recommended',
    parserOptions: {
        ecmaVersion: 6,
        sourceType: 'module'
    },
    // rest...
};

and the ESLint just breaks on async keyword. Which was defined in ES2017 (8) while we're on ES2015 (6). So while we're adding the space-before-function-param support for async it will not work due to lower ECMA version.

~Edit: I've it work also with 7 as the async/await was also in ES2016 (I don't get the sources right apparently...)~ not true. The 8 is minimal to async/await support is ESLint.

[jodator]

We probably need to upgrade the ecmaVersion to at least 9 as we're using 7-9 features already. cc @Reinmar

[ma2ciek]

We use it outside of the ckeditor5 repository, e.g. in CF with the higher ecmaVersion option set. But yeah, upgrading the default ecmaVersion prop does make sense too.

[jodator]

So either this should go to overridden config or we should update it here already.

I'm for the latter to set it to 2018 (or 9).

[jodator]

@Reinmar & @mlewand: quick summary of features used:

• ES 2018:
  
  
  
  • RegExp enhancements (not linted, feature detection is used)
  
  
• ES 2017:
  
  
  
  • Object.values() (my bad - tables)
  
  
  • Object.entries() (tables, remove format)
  
  
• ES 2016:
  
  
  
  • Array.includes() (basically everywhere)
  
  

So minimum update is 2016, but updating to 2018 shouldn't introduce any side effects.

My vote is to bump this to to 2018 & close @ma2ciek PR.

[Reinmar]

👍