Update linter to enforce ES features

[CodyJasonBennett]

Description

We'll want to update the linter (#23763) to ensure that ES2018 syntax is used consistently in both source and addons (see #25172). Currently it accepts a browserslist rather than an ES version to ensure that experimental browser APIs are used responsibly. This doesn't replace discussion around the use of those features, though.

Solution

I'd like to replace or add on (preference for the first) a linter plugin that checks against ES features only. This is a common enough thing for me to be comfortable writing one on top of a parser if we can't find something well supported. As this widely affects quality control, this should have great language testing coverage before running wild here.

Alternatives

Transpilation was mentioned in #24237, but discarded in favor of lighter to no tooling. I'll want to echo that in choice of linter tooling around this issue.

Additional context

I haven't considered updating browerslist as per evanw/esbuild#121 (comment) since that would give false positives for browser APIs. As this is a moving target, I don't think it's appropriate to update mdcs.

[epreston]

I have a few questions:

The goal is to enforce 2018 syntax across all source and examples ?

The plugin eslint-config-mdcs enforces 2020 syntax for the last 3 years, does that align ? Is it still maintained, will you keep the dependency ? The rules can be copied over if you want to keep the functionality but drop the dependency.

A few observations:

I didn't find an ecmaVersion defined in the eslintConfig section of the package.json. Ommitting the other settings for clarity, it should look like:

  "eslintConfig": {
    "parserOptions": {
      "ecmaVersion": 2018
    },
  },

If you set eslint to 2018, it correctly errors on line 89 of Three.js, because it is 2020 syntax as shown below.

export * as AnimationUtils from './animation/AnimationUtils.js';

Some options:

• Set ecmaVersion to 2020 and be done with it. It's 2023, existing tools are enforcing 2020, code has 2020 features, 'optional chaining' could simplify lots of the code base (see below). If the project has been shipping this without complaints, you might be in safe territory.

• ecmaVersion version to 2018 and update Three.js explicitly export AnimationUtils classes.

What are your thoughts ? We can make lots of progress on this with a very little code.

Optional Chaining: (Taken from Wikipedia)

Optional chaining makes it possible to access the nested properties of an object without having an AND check at each level. An example is const zipcode = person?.address?.zipcode. If any of the properties are not present, zipcode will be undefined.

[donmccurdy]

I think the idea is that the build process will compile the imports and exports appropriately — there's only one file in the bundle at the end of the day, that's Rollup's job. The rest of the codebase, excluding imports and exports, should use ES2018. Is that an option we can configure ESLint for?

My general feeling is that we don't really need features like nullish coalescing right now, the DX improvement isn't meaningful at this time, and some tools have broken when these features have gone into the build accidentally. We just want to prevent that from happening again — using ES2020 is a non-goal for now.

[epreston]

@donmccurdy yes, just add the setting above to your package.json with a value of 2018. The Linter's parser fails at language features above that. It overrides any plugins that are specifying something else. In turn people get warnings in their tools and the PR's are checked against 2018 before review.

As far as the build tools go, the project is bundling for a brower support target not a language support target. The syntax of the bundle is determined by this dynamic setting which changes every quarter:

  "browserslist": [
    "> 1%, not dead, not ie 11, not op_mini all"
  ],

I'm happy to take the time to translate that back to language level (plus or minus) language features.

[epreston]

You have pointed out options, you can go 2020 and exclude the features you want to avoid with a few quick rules. The parser option used above is kind of a hard upper bound that we can retreat from.

[epreston]

Writing this out for clarity so I can build a PR today that meets this 150 release goal:

• Developers write 2018 code without any of the bug prone language features that you want to avoid.
• Keep the eslint-config-mdcs plugin because it only contains style features and we can override the permissive language level specified in it's env config using line in the projects package.json as above.
• Build continues to bundle for browser target (which may contain any set of language features to meet it's goals)
• If there's anything meaningful in more recent language levels (handy import syntaxes like in the example above) , implement as 2020 stripped backwards, if not: implement as 2018 and remediate any linter issues.

How's that sound ?

[donmccurdy]

I'm hoping for an ESLint configuration that warns us about use of features newer than ES2018, except import/export statements which may use more recent syntax.

[epreston]

It's on the way.

[epreston]

I need to do this in two stages.

• First is the single line to enforce 2018, and fix those 3 import statements.
• Second is relax it to 2020 parse mode, and rules to strip it back.

[marcofugaro]

I'm hoping for an ESLint configuration that warns us about use of features newer than ES2018, except import/export statements which may use more recent syntax.

After some tests by @epreston, the best way would be to achieve this result would be to use ES2020 + eslint-plugin-escompat with the rules no-nullish-coalescing and no-optional-chaining.

I believe the other features from ES2019/ES2020 are not that relevant to this repo.

[epreston]

Take some more time to look into it a bit more. There's lots of fast opinions and people don't read everything carefully.