Update: support .eslintrc.cjs (refs eslint/rfcs#43)

[evanplaice]

In ES module packages w/ "type": "module" defined treat all .js files as ES modules. Since eslint is expecting the config in CommonJS format, it needs to be named w/ a .cjs extension.

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

For details about the issue, see #12319.
For details about the resolution, see RFC #43

What changes did you make? (Give an overview)

Map the .cjs file extension for loading CommonJS configurations (ie .eslintrc) in ES module based packages.

Is there anything you'd like reviewers to focus on?

After searching through the source, I didn't see any tests that cover this code. Is there an existing test I need to add/update to cover this functionality?

Also, what section of the docs should ES module usage be documented?

[kaicataldo]

Thanks for the PR. We ask that changes to core go through the RFC process outlined here. Do you mind doing that?

Incidentally, I don't think we should consider this a bug fix. It would be an enhancement because it would be adding support for an experimental Node feature.

[mysticatea]

Yeah, this is not a bug fix.

I think too early to merge this feature because ES modules on Node.js is still experimental and it may change due to the large impact to JS ecosystem.

[evanplaice]

Speaking from experience, any conversation discussing modules + node has a tendency to fall down the rabbit hole. I posted this PR so there's an actual concrete example to look at.

I don't expect this PR to land quickly, if at all.

@kaicataldo

We ask that changes to core go through the RFC process outlined here. Do you mind doing that?

Not at all, it'll take me a bit to gather and record my thoughts

---

@mysticatea

Of course. The initial issue said 'bug' so I followed that workflow. I'll fix it once I get a feel for the process.

As far as the time schedule goes -- if nothing major has or will change between now and then -- modules will be officially released (ie unflagged) on 20 October. There's still time to discuss, bike shed, shave a few yaks, and decide the best approach for ESLint specifically.

[platinumazure]

This looks good to me at first glance (so far).

I'm a bit surprised there aren't any tests. I guess I would like to see if we could add a new test for the CJS case at least, to avoid regressions.

No need to make any immediate changes, as the RFC is still being discussed. This is just a preliminary review from me. 😄

[evanplaice]

Update: As mentioned in this issue it looks like the schedule for unflagging ES modules is going to be pushed a bit.

[jkrems]

Update: The unflagging PR (nodejs/node#29866) has now landed, an unflagged version of module support in node should be part of the next 13.x release next week.

[mysticatea]

Hi. Thank you for working on this.

RFC43 has been merged in the last week.
Would you update this PR to follow the RFC?

[kaicataldo]

Do you mind adding some tests? You can find relevant tests in https://github.com/eslint/eslint/blob/master/tests/lib/cli-engine/config-array-factory.js.

[evanplaice]

I added test cases to:

• "'loadFile(filePath, options)' method should load a config file."
• "'loadInDirectory(directoryPath, options)' method should load the config file of a directory."
• "'extends' property should handle the content of extended configs properly."

And added the test:

• "should load information from a JavaScript file with a .cjs extension"

As well as the fixture:

• /eslint/tests/fixtures/config-file/cjs/.eslintrc.cjs

Is that sufficient? AFAIK, these are the tests that cover config loading. The rest of the tests appear to dig into specific JavaScript-configuration behaviors which should be no different between .js vs .cjs.

[evanplaice]

The CI / Browser Test (pull_request) is failing across the board for all PRs.

Should I squash + rebase after it's fixed or just leave it?

[kaicataldo]

CI / Browser Test is failing due to #12662 and unrelated to this PR. We're trying to decide on a fix. We shouldn't hold up merging this for that.

[kaicataldo]

This LGTM. Thank you for working on this!

[evanplaice]

@kaicataldo I'm happy to help. Big thanks to you and everybody else who was involved in this for your patience.

[platinumazure]

Looks good, thanks for contributing!

[mysticatea]

LGTM. Thank you!

[kaicataldo]

Thanks for contributing to ESLint!