Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Feedback Request: ESLint ES Module Compatibility #447

Closed
evanplaice opened this issue Nov 26, 2019 · 12 comments
Closed

Feedback Request: ESLint ES Module Compatibility #447

evanplaice opened this issue Nov 26, 2019 · 12 comments

Comments

@evanplaice
Copy link

evanplaice commented Nov 26, 2019

New: ES Module Compatibility

The RFC to add ES module compatibility support to ESLint is ready. Feedback is welcome.

Summary

Currently, .eslintrc.js files are defined using the Common JS module format. When included in an ESM package (ie type: module) using the .js extension Node will (correctly) throw an error. The fix for this is to rename .eslintrc.js -> .eslintrc.cjs so Node will correctly load the file as a Common JS module.

This RFC identifies the reasoning as well as the modifications necessary to make ESLint recognize .cjs as a valid module format.

Scope

This does not cover defining the config in ES module format. For now, there are no plans to add support for that approach.


Is this a viable resolution for...

@evanplaice evanplaice changed the title Feedback Request: ESLint ES module compatibility Feedback Request: ESLint ES Module Compatibility Nov 26, 2019
@coreyfarrell
Copy link
Member

This is similar to my current plan for nyc, the only difference is that nyc will support ESM in .mjs config files. .js and .cjs configs will be loaded using require unconditionally.

I plan to reevaluate this when it is possible to detect the format of .js or to use import() without risk of an experimental warning.

@evanplaice
Copy link
Author

@coreyfarrell That's good to hear 👍

Quick question, How do you get .js to load as a Common JS module from within a ES package without error?

@coreyfarrell
Copy link
Member

@evanplaice I didn't / it fails. Users with "type": "module" cannot use .js config files for nyc at all, they will have to use .cjs or .mjs. We likely don't have that many users actually using config scripts as even nyc.config.js is a relatively new feature, most users likely have json config.

@DerekNonGeneric
Copy link
Contributor

DerekNonGeneric commented Nov 27, 2019

/to @evanplaice

Quick question, How do you get .js to load as a Common JS module from within a ES package without error?

Can't you use createRequire like this? As far as feedback, I'm not opposed to ESLint's config remaining with CJS entry points. It will not affect my current setup of writing my ESLint config in ESM format because I am currently doing as is described in the following Stack Overflow post.

ES2015 syntax for default exports in an ESLint config file

@Jamesernator
Copy link

@coreyfarrell See for discussion on detecting package/file type: nodejs/node#30514

@evanplaice
Copy link
Author

@DerekNonGeneric No, for 2 reasons

  1. The change can't break backward compatibility. Assume ESLint will have to support Node 8+ users.

  2. ESLint already uses a custom resolver (ie import-fresh) to circumvent Node's built-in caching mechanisms. An additional custom resolver would need to be made compatible.

Custom resolvers are cool but they introduce additional complexity that may break userland in unforseen/unexpected ways.

@DerekNonGeneric
Copy link
Contributor

I see what you're saying about it not being feasible due to needing to support Node 8+ since module.createRequire() was added in v12.2.0. Thanks for mentioning import-fresh. I've been meaning to study how ESLint is able to load multiple formats. Lack of control over the caching mechanism seems to be a recurring theme. I hope it's not a dealbreaker for most.

@GeoffreyBooth
Copy link
Member

How do you get .js to load as a Common JS module from within a ES package without error?

See eslint/eslint#12333

@evanplaice
Copy link
Author

evanplaice commented Nov 27, 2019

Interesting, I missed most of that convo before. So, it's possible but complex to implement. I wonder how difficult it would be to package that solution in a way for other projects to use since this is going to be a common issue.

@GeoffreyBooth
Copy link
Member

Interesting, I missed most of that convo before. So, it's possible but complex to implement. I wonder how difficult it would be to package that solution in a way for other projects to use since this is going to be a common issue.

That PR is more of a hack than anything I’d recommend. I think they should go the route of the other PR, and start supporting .cjs (and/or ESM config .js/.mjs files).

@coreyfarrell
Copy link
Member

My config loading process already uses find-up so in theory I could use that to find the package.json relative to the selected config file when dealing with .js, load the package.json to find the type. I'm unlikely to do this as I'd be stuck with it for a really long time. nodejs/node#30514 will hopefully provide a solution so I'm subscribed to that issue. Without that eventually ESM will no longer be experimental, once that's the case I can check for node.js version to decide if import() can be used on .js files.

@evanplaice
Copy link
Author

evanplaice commented Dec 3, 2019

The RFC landed successfully. Big thank you to everybody who participated.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants