Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upGlobals configuration is unintuitive #9940
Comments
not-an-aardvark
added
enhancement
core
evaluating
labels
Feb 4, 2018
This comment has been minimized.
This comment has been minimized.
As you correctly mentioned, this is due to legacy support for JSHint. I remember when it was implemented, there was some discussion about it, but that was pretty early on, and we decided to follow JSHint notation (seeing how it would help people migrate from JSHint without having to update all of their files manually). |
This comment has been minimized.
This comment has been minimized.
I'd like to see this get implemented in 5.x. Curious to see what the rest of @eslint/eslint-team thinks. |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
I don't feel strongly either way, but I would speculate that configuration keywords might be slightly easier to use when they don't have internal punctuation, because they would be easier to understand when spoken aloud (without needing to say something like "read dash only"), and they would be consistent with our other configuration keywords (none of which have punctuation). There is precedent for using the term A third option would be to just use the word |
ilyavolodin
added
accepted
and removed
evaluating
labels
Jun 26, 2018
This comment has been minimized.
This comment has been minimized.
I'll mark it as accepted, since this has been up-voted by 5 members of TSC. In terms of the name, I would vote for either |
added a commit
to aladdin-add/eslint
that referenced
this issue
Aug 16, 2018
added a commit
to aladdin-add/eslint
that referenced
this issue
Aug 17, 2018
kaicataldo
added
help wanted
Hacktoberfest
labels
Oct 15, 2018
This comment has been minimized.
This comment has been minimized.
@aladdin-add it looks like you started working on this. Are you still working on this? |
not-an-aardvark commentedFeb 4, 2018
Previous issues: #4734, #5765
Background
Currently (ESLint v4.17.0), the
globals
value in a config file allows a user to configure globals for their project. The configuration looks like this, where each global is given a boolean value:Similarly, a boolean value can be used in a
/* global */
comment directive to enable globals for a file:/* global Foo: true, Bar: false, baz: false */
A value of
true
indicates that a global is allowed to be rewritten, and a value offalse
indicates that a global is read-only. This behavior is inherited from JSHint and JSLint.Problem
There are a few disadvantages to the current behavior:
true
(rewritable) andfalse
(read-only). This will make it more complex to enable a set of globals by default in the future (e.g. when we start supporting ES6 by default), because users will not be able to disable the globals.true
/false
and "rewritable"/"read-only" is somewhat arbitrary, which makes it confusing to users. Anecdotally, I had been on the ESLint team for almost a year before I learned that/* global Foo: false */
would makeFoo
read-only. (I had previously thought that it would turn offFoo
.)The root cause of these problems is that there are three possible states of a global ("not present", "present but read-only", and "present and writable") but we only allow them to be configured with a boolean field.
Proposal
We should start allowing string values for globals. The possible values are (bikesheddable):
readonly
to indicate that a global is read-only (same asfalse
)writable
to indicate that a global is writable (same astrue
)off
to indicate that a global is not presentTo encourage people to use the new forms, we should deprecate the use of
true
andfalse
for configuring globals. (However, for backwards compatibility,true
andfalse
would continue to be supported without a warning for the foreseeable future.)