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

no-undef error for 'module' in created .eslintrc.js when using ESM and not-Node #59

Closed
JoshuaKGoldberg opened this issue Apr 16, 2023 · 6 comments · Fixed by #66
Closed

Comments

@JoshuaKGoldberg
Copy link

JoshuaKGoldberg commented Apr 16, 2023

I ran this creator with as few plugins / extra points as possible, with esm as the module selection:

$ npm init @eslint/config

✔ How would you like to use ESLint? · problems
✔ What type of modules does your project use? · esm
✔ Which framework does your project use? · none
✔ Does your project use TypeScript? · No / Yes
✔ Where does your code run? · No items were selected
✔ What format do you want your config file to be in? · JavaScript
Local ESLint installation not found.
The config that you've selected requires the following dependencies:

eslint@latest
✔ Would you like to install them now? · No / Yes
✔ Which package manager do you want to use? · npm

...

A config file was generated, but the config file itself may not follow your linting rules.
Successfully created .eslintrc.js file in /Users/josh/repos/eslinttemp

The config file itself not following the linted rules is something that I've seen confuse newcomers repeatedly. Example: typescript-eslint/typescript-eslint#6825

Since no-undef will hit roughly every project that uses ESM, is putting an /* eslint-env node */ on top of the config (if it would violate no-undef for module.exports = ...) reasonable?

I also understand the new flat config is going to take over within the next 1-5 years. A lot of people still use the current config, so it'd be nice to have it be less confusing to them in the interim.

Edit: to be clear, what I found confusing is that this ESLint error is caused by running ESLint with the config generated by init:

'module' is not defined. eslint [no-undef](https://eslint.org/docs/rules/no-undef)
@nzakas
Copy link
Member

nzakas commented Apr 18, 2023

Since no-undef will hit roughly every project that uses ESM, is putting an /* eslint-env node */ on top of the config (if it would violate no-undef for module.exports = ...) reasonable?

I don't think so. Unless someone explicitly says that the JavaScript is intended to run on Node.js, adding a whole environment might cause problems. We could add /* global module */, though.

I also understand the new flat config is going to take over within the next 1-5 years. A lot of people still use the current config, so it'd be nice to have it be less confusing to them in the interim.

I'm not sure I understand the context of this comment. Soon, we'll switch --init to generate a flat config file so this won't be an issue. And this command is used only rarely when people start a new project (even then, they are probably copying from an existing project instead), so I'm not sure how the people who are using the current config now would be affected.

@JoshuaKGoldberg
Copy link
Author

JoshuaKGoldberg commented Apr 18, 2023

Unless someone explicitly says that the JavaScript is intended to run on Node.js
adding a whole environment might cause problems

Oh, is it not the case that .eslintrc.js files are explicitly made to run in Node.js? I was under the impression ESLint's current config system is designed for Node.js. Assuming that's correct - I don't follow how this could cause problems. If .eslintrc.js is made to run in Node.js, does adding /* eslint-env node */ to the top of .eslintrc.js impact any other files?

An example target project could be no environment for source files in general but still Node for .eslintrc.js.

the context of this comment

I was worried that you / the ESLint team would see that this issue is for the legacy config and wontfix it because --init will soon switch to generating a flat config. So I wanted to point out that this is still a command used not-rarely when people start a new project. 😄

And this command is used only rarely when people start a new project

...huh. I find that to be a surprising comment. Given that npm init @eslint/config is on the homepage of https://eslint.org I don't see how that could be the case for projects not managed by an external framework builder (e.g. Next.js).

Edit: added clarification to the OP

@nzakas
Copy link
Member

nzakas commented May 1, 2023

Oh, is it not the case that .eslintrc.js files are explicitly made to run in Node.js?

Fair point, got my wires crossed a bit. Still, we are phasing out the use of environments, so don't want to start introducing them in new places.

...huh. I find that to be a surprising comment. Given that npm init @eslint/config is on the homepage of https://eslint.org/ I don't see how that could be the case for projects not managed by an external framework builder (e.g. Next.js).

Most people don't start projects from scratch. They end up joining a project already in progress or, if they actually are starting from scratch, they tend to just copy over existing configuration rather than running --init.

@JoshuaKGoldberg
Copy link
Author

JoshuaKGoldberg commented May 1, 2023

phasing out the use of environments

TIL. Are there discussion notes somewhere explaining more on that? Any formal plans around deprecation?

Most people don't start projects from scratch. ... joining a project already in progress

Just making sure we're talking about the same thing: I'm attempting to report a seeming-bug in npm init @eslint/config. Even if most people don't start projects from scratch, this is still a bug that some users will see -> should be fixed, right?


Taking a step back from @eslint/config: I've seen a good number of people -myself included- hit the no-undef error for module when trying to either create or update an ESLint config. Is there guidance from the ESLint team on the right way to solve this? E.g. /* global module */ as posted?

Note that in typescript-eslint/typescript-eslint#6825 -> typescript-eslint/typescript-eslint#6918 we added /* eslint-env node */ to https://typescript-eslint.io/getting-started. So it'd be great to know whether we got that wrong. 😄

@nzakas
Copy link
Member

nzakas commented May 9, 2023

TSC meeting transcripts and notes are available here: https://github.com/eslint/tsc-meetings

Just making sure we're talking about the same thing: I'm attempting to report a seeming-bug in npm init @eslint/config. Even if most people don't start projects from scratch, this is still a bug that some users will see -> should be fixed, right?

Yes. I'm just saying that because environments are going away, I'd prefer the fix to just add the missing global rather than adding in the environment.

@nzakas
Copy link
Member

nzakas commented May 9, 2023

Oops, also announced that environments are going away here: https://eslint.org/blog/2022/08/new-config-system-part-2/#goodbye-environments%2C-hello-globals

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

Successfully merging a pull request may close this issue.

2 participants