eslint: Change Request: `/* exported */` should not allow values

Let’s also keep in mind that usage of /*exported*/ is likely incredibly low at this point. Both CommonJS and ESM use syntax to mark exports, so we’re really talking about a minority of people using some other module system and needing to mark var declared variables so we don’t complain that they are unused.

Given this, and #17622 (comment), I think it’s fine that we just drop support for /*exported a:true*/ and /*exported a b*/ without any warnings. If we ever switch to a format that is clearly eslint-specific (e.g., /* eslint-exported a */) then we could emit warnings for comments that have no effect.

Sounds good to me.

So to summarize, it seems like we’ve agreed that we should only support /*exported a, b*/, and we should not emit a warning for any other syntax.

@nzakas @mdjermanovic I apologize for the delay in my response, which was due to personal reasons. I’ll update the PR along with @injae-kim soon!

OpenSource Study at GDG songdo, south korea (Help developers to make first PR on opensource they want)

JFYI) Hi @nzakas , @mdjermanovic , I’m OpenSource Study leader(also contributor) and helping @amondev to fix this issue!

Cause @amondev has been busy these days for some personal reason, I’ll ping to @amondev and update PR based on your discussion within next weekend~!

Really thanks a lot for your detailed discussions. 🙇

But parseListConfig doesn’t (values must be comma-separated), so /* exported a b */ will also become invalid/no-effect if we switch to parseListConfig.

If we don’t have any tests validating /* exported a b */ then I think it’s fine to drop support for that. Arguably, forcing commas would reduce the likelihood that we’d collide with a comment that wasn’t intended to mark a variable as exported.

Let’s also keep in mind that usage of /*exported*/ is likely incredibly low at this point. Both CommonJS and ESM use syntax to mark exports, so we’re really talking about a minority of people using some other module system and needing to mark var declared variables so we don’t complain that they are unused.

I just realized that parseStringConfig also allows space-separated values:

/* eslint no-unused-vars: "error" */

/* exported a b */

var a, b; // both valid

But parseListConfig doesn’t (values must be comma-separated), so /* exported a b */ will also become invalid/no-effect if we switch to parseListConfig.

So sounds like we want to check that a given exported variable is valid, and if not, emit a warning.

@amondev we probably want to use a regular expression, something like /[^a-z$_][^a-z0-9$_]*$/ on each variable name, and if it fails, emit a warning (similar to this.)

So, we probably want to check for that case and avoid creating that variable, plus emit a warning?

Makes sense to check for that case, since it’s currently valid so warnings might help with fixing the code for v9.

In the future, we could maybe add a more general check that would itself include this specific one, like whether the parsed value is a valid variable name or if the variable was actually found (/* exported */ doesn’t create variables; it just sets properties on existing ones and ignores non-existing ones).

https://github.com/eslint/eslint/blob/994596b07f5ff20a615a4be1ea03e5fd59cdb84b/lib/source-code/source-code.js#L308-L319

One thing we still need to decide on: what should happen if we see /* exported foo: true */? If we just change to use parseListConfig(), then we will be creating variables with the name “foo: true”.

So, we probably want to check for that case and avoid creating that variable, plus emit a warning?