Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task done
simonhaenisch opened this issue Aug 5, 2024 · 9 comments
Open
1 task done

Comments

@simonhaenisch
Copy link

simonhaenisch commented Aug 5, 2024

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

  • I am willing to submit a pull request for this change.

Additional comments

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

@scagood
Copy link

scagood commented Aug 5, 2024

I think I agree here.


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

playground example


Thoughts @aladdin-add and @voxpelli?

This is also possibly related to #300

@voxpelli
Copy link
Member

voxpelli commented Aug 5, 2024

Yeah, I think #300 should be the recommended one

@voxpelli
Copy link
Member

voxpelli commented Aug 5, 2024

@simonhaenisch
Copy link
Author

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.

@ohadschn
Copy link

ohadschn commented Dec 20, 2024

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
]

@voxpelli , @scagood , @simonhaenisch
What would be the equivalent workaround for pure ESM projects (not mixed)?
The same but use recommended-script instead of mixed-esm-and-cjs?

I'm also a bit confused about the spread operator (...) used in the workaround above, the docs don't include it:

const nodePlugin = require("eslint-plugin-n")

module.exports = [
    nodePlugin.configs["flat/recommended-script"],
    {
        rules: {
            "n/exports-style": ["error", "module.exports"]
        }
    }
]

@aladdin-add
Copy link

for pure esm projects (with pkg.type = "module"), just use recommneded (and it's just the same as recommended-module):

import node from 'eslint-plugin-n'

export default [
 node.configs['flat/recommended']
]

@ohadschn
Copy link

for pure esm projects (with pkg.type = "module"), just use recommneded (and it's just the same as recommended-module):

import node from 'eslint-plugin-n'

export default [
 node.configs['flat/recommended']
]

ohh so that's what the script suffix meant there... thanks!
Perhaps consider clarifying the docs a bit

@aladdin-add
Copy link

sure, PRs are always welcome! 😄

@ohadschn
Copy link

ohadschn commented Dec 20, 2024

sure, PRs are always welcome! 😄

Forgive me but I barely know what I'm doing here (not a JS dev to say the least).
Took me a while to even get this config working, can't say I understand the difference between your syntax, the docs syntax, and the syntax which finally worked for me (in the previous two forms, the rules didn't actually run, I injected offenders to test):

import pluginN from 'eslint-plugin-n';

export default [
  {
    files: ['**/*.js'],
    languageOptions: {
      globals: globals.node,
      sourceType: 'module',
    },
    plugins: {
      n: pluginN,
    },
    rules: {
      ...pluginN.configs['flat/recommended'].rules,
    },
  },
];

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

No branches or pull requests

5 participants