New: Allow globals to be disabled/configured with strings (fixes #9940) #11338
Conversation
not-an-aardvark
added
enhancement
not-an-aardvark
force-pushed
the
string-globals
branch
from
09db38f
to
63672ed
Compare
As described in #9940, this allows globals in a config file to be specified with the strings `readable` and `writeable` rather than `false` and `true`, respectively. It also adds a new value, `off`, which turns off a previously-enabled global.
not-an-aardvark
force-pushed
the
string-globals
branch
from
63672ed
to
bce42ee
Compare
| @@ -208,19 +208,19 @@ To specify globals using a comment inside of your JavaScript file, use the follo | |||
| /* global var1, var2 */ | |||
| ``` | |||
|
|
|||
| This defines two global variables, `var1` and `var2`. If you want to optionally specify that these global variables should never be written to (only read), then you can set each with a `false` flag: | |||
| This defines two global variables, `var1` and `var2`. If you want to optionally specify that these global variables can be written to (rather than only being read), then you can set each with a `writeable` flag: | |||
There was a problem hiding this comment.
This documentation was previously misleading -- it implied that "writeable" was the default for global comments like /* global foo */, but in fact the default seems to be "readable".
|
|
||
| // Fallback to minimize compatibility impact | ||
| default: | ||
| return "writeable"; |
There was a problem hiding this comment.
Variable writability is communicated to rules via the variable.writeable boolean property in scope analysis.
Previously, a non-boolean value in a globals configuration would actually get passed directly to rules through scope analysis, so a configuration of {globals: {myGlobal: "foo"}} would end up as a variable object with variable.writeable = "foo".
Strictly speaking, we don't have any compatibility guarantees for non-boolean values here, since it's undocumented behavior. But since someone is probably relying on this by mistake (e.g. from a malformed config comment), I put in a fallback so that unexpected globals values default to "writeable" (and appear to rules as writeable: true). This fallback seems most likely to match existing behavior when users have malformed configs, since rules using this would typically be doing truthiness checks.
docs/user-guide/configuring.md
Outdated
| ``` | ||
|
|
||
| These examples allow `var1` to be overwritten in your code, but disallow it for `var2`. | ||
|
|
||
| For historical reasons, the boolean values `false` and `true` can also be used to configure globals, and are equivalent to `"readable"` and `"writeable"`, respectively. However, this usage of booleans is deprecated. |
There was a problem hiding this comment.
Not a blocker: Do we have a way to stylistically indicate that this is an aside? (I'm assuming not and that this would be a wider information architecture question, but wanted to ask.)
Also: I wonder if this should go below the next section on disabling globals? (Edit: Or another possibility might be to use subsections for enabling globals [read/write], and disabling [off], in which case the paragraph probably shouldn't move, but everything would flow a little better because the subsection headers would serve as natural breaks.)
There was a problem hiding this comment.
I moved this line below the section about disabling globals.
I think this line will be useful for users figuring out what's going on in existing configs containing booleans, so I don't want to make it too much of a footnote. I was thinking about preceding this line with "Note:", but in the new location this line is followed by another line that also starts with "Note:", which would look a bit strange. That said, I'm open to stylistic suggestions.
Co-Authored-By: not-an-aardvark <teddy.katz@gmail.com>
Co-Authored-By: not-an-aardvark <teddy.katz@gmail.com>
Previously, invalid values for `globals` configurations would default to `writable`. This change updates those cases to throw an error instead.
eslint-deprecated
bot
added
the
archived due to age
What is the purpose of this pull request? (put an "X" next to item)
[x] Add something to the core
What changes did you make? (Give an overview)
As described in #9940, this allows globals in a config file to be specified with the strings
readableandwriteablerather thanfalseandtrue, respectively. It also adds a new value,off, which turns off a previously-enabled global.Is there anything you'd like reviewers to focus on?
Nothing in particular