Change Request: Don't set `languageOptions.sourceType` (for ESLint 9)

simonhaenisch commented 1 month ago

eslint-plugin-n version

17.10.2

What problem do you want to solve?

https://eslint-online-playground.netlify.app/#eNpVjjsOwjAQRK9ibZMGzKfh13ILlsKy15HBXltxQJGi3J04SUHanfd2pofc6B11KiRPMrwyXMGFFJtW2CxsE4OobK5gA5S941bqyNbVa5KjoZnFBdsm/6kdbxnhhoxM3QQasurjW/FAFpO1vMuPynrV7hrSMQRiQ6Z6Ij9Hd2xOSr9VTfKVI4+lfZERDH3vlArL2lFGuIopKdk8opwQLvIs9wibdfY3sECHkzzs5RGhUAPyAMMPStleUQ==

☝️ gives

Parsing error: 'import' and 'export' may appear only with 'sourceType: module'

even though the file is using .mjs extension. After some digging I found the problem is this plugin's config which enforces languageOptions.sourceType: 'commonjs' for all files. ESLint 9 already sets the sourceType correctly based on type field in package.json and file extensions (.js/.cjs/.mjs), so there's no need for this plugin to configure this.

What do you think is the correct solution?

Don't set sourceType in language options and let ESLint handle this instead... see this playground where I removed the field from the config object and then it works fine.

https://eslint-online-playground.netlify.app/#eNpdkDFPAzEMhf9K5OWWNm1ZgCLEwo6E2GqGKHFOKYkTXXKoqLr/ziVXCdrV73vPfj5DHvSGTiokTzIcM+zBhRSHImwWdohBdDZ3sALK3nGROrJ1/TXJ0dDC4gVbJz/2jteM8ISMPLvywr2TjiEQGzLiuU0ukfnQWa/KZvgDus9mNuSp0K1besX9qHp6S8XN8S8yx3HQ9PGTqNno1I4zZNXoizggi9sM5LphLpeU/pqj5DFHnnudK4tg6PuVUiVZO8oIe9GUqi096wjhUT7ILcLqWvv3gwrt7uVuK+8QKjUhTzD9AiiihF8=

Participation

[X] I am willing to submit a pull request for this change.

Additional comments

Not sure if it's the same for ESLint 8.

scagood commented 1 month ago

I think I agree here.

Maybe its worth making the flat/mixed-esm-and-cjs config the recommended one :thinking:

playground example

Thoughts @aladdin-add and @voxpelli?

This is also possibly related to #300

voxpelli commented 1 month ago

Yeah, I think #300 should be the recommended one

voxpelli commented 1 month ago

I'm running something similar to #300 in my own projects: https://github.com/voxpelli/eslint-config/blob/9eb2261fd1e0b91bea570bd83811911cf23849f6/base-configs/node.js#L12

simonhaenisch commented 1 month ago

Just tried

import node from 'eslint-plugin-n'

const nodeRecommended = node.configs['flat/mixed-esm-and-cjs']

nodeRecommended.forEach(config => delete config.languageOptions?.sourceType)

export default [
 ...nodeRecommended,
 // more stuff
]

and that works fine for me (.js files based on type field in package.json, and .cjs/.mjs fixed to CommonJS/ESM respectively).

So I'm also for making mixed-esm-and-cjs the actual recommended config.

I'd also drop the languageOptions.sourceType config, however I'm not sure if ESLint 8 also sets that correctly already.

eslint-community / eslint-plugin-n