Skip to content

Commit aacdd35

Browse files
authored
Rule fix: Add allowIds option to no-global-selector to allow global selector by ID
* Allow global selector by ID Selection by ID is safe, we should allow it. I believe that's what was originally intended in this rule? Should the same fix be applied to the `context` selector below? * Adjust test to allow global ID selectors * Make no-global-selector allowIds an option
1 parent 953d0c5 commit aacdd35

File tree

3 files changed

+57
-6
lines changed

3 files changed

+57
-6
lines changed

docs/no-global-selector.md

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
# no-global-selector
22

3-
Disallows global selectors which search the whole document. Encourages users to keep references to DOM nodes in memory, instead of selecting them from the DOM each time.
3+
Disallows global selectors which search the whole document. Encourages users to keep references to DOM nodes in memory, instead of selecting them from the DOM each time. Use the `allowIds` option to allow single ID selectors.
44

55
## Rule details
66

77
❌ The following patterns are considered errors:
88
```js
9-
$( '.div' );
9+
$( 'div' );
1010
$( '#id' );
1111
$( '.selector' );
1212
$( '.selector > .child' );
@@ -16,6 +16,12 @@ $( '.selector', null );
1616
$( '.selector', undefined );
1717
$( '.selector', $( '.context' ) );
1818
```
19+
❌ With `[{"allowIds":true}]` options:
20+
```js
21+
$( '#id>div' );
22+
$( '#id~div' );
23+
$( '#id div' );
24+
```
1925

2026
✔️ The following patterns are not considered errors:
2127
```js
@@ -37,6 +43,11 @@ $( undefined );
3743
$( false );
3844
$( '#' );
3945
```
46+
✔️ With `[{"allowIds":true}]` options:
47+
```js
48+
$( '#id' );
49+
$( '#id-foo_bar1' );
50+
```
4051
## Rule source
4152

4253
* [rules/no-global-selector.js](../src/rules/no-global-selector.js)

src/rules/no-global-selector.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,28 @@ const utils = require( '../utils.js' );
44

55
// HTML regex (modified from jQuery)
66
const rquickExpr = /^(?:\s*(<[\w\W]+>)[^>]*)$/;
7+
// ID patterns that return one DOM node
8+
const idPattern = /^#[^>~\s]+$/;
79

810
module.exports = {
911
meta: {
1012
type: 'suggestion',
1113
docs: {
12-
description: 'Disallows global selectors which search the whole document. Encourages users to keep references to DOM nodes in memory, instead of selecting them from the DOM each time.'
14+
description: 'Disallows global selectors which search the whole document. ' +
15+
'Encourages users to keep references to DOM nodes in memory, instead of selecting them from the DOM each time. ' +
16+
'Use the `allowIds` option to allow single ID selectors.'
1317
},
14-
schema: []
18+
schema: [
19+
{
20+
type: 'object',
21+
properties: {
22+
allowIds: {
23+
type: 'boolean'
24+
}
25+
},
26+
additionalProperties: false
27+
}
28+
]
1529
},
1630

1731
create: function ( context ) {
@@ -26,10 +40,13 @@ module.exports = {
2640
return;
2741
}
2842
const value = node.arguments[ 0 ].value;
43+
const allowIds = context.options[ 0 ] && context.options[ 0 ].allowIds;
2944
if (
3045
typeof value !== 'string' ||
3146
!value ||
32-
value === '#' ) {
47+
value === '#' ||
48+
( allowIds && idPattern.test( value.trim() ) )
49+
) {
3350
return;
3451
}
3552

tests/rules/no-global-selector.js

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ ruleTester.run( 'no-global-selector', rule, {
1616
'$(".selector", context)',
1717
'$(".selector", "#")',
1818
'$(".selector", [])',
19+
{
20+
code: '$("#id")',
21+
options: [ { allowIds: true } ]
22+
},
23+
{
24+
code: '$("#id-foo_bar1")',
25+
options: [ { allowIds: true } ]
26+
},
1927
'$(function() {})',
2028
// Variables could be a selector, but could equally be HTML or functions, so leave as valid
2129
'$(variable)',
@@ -31,7 +39,7 @@ ruleTester.run( 'no-global-selector', rule, {
3139
],
3240
invalid: [
3341
{
34-
code: '$(".div")',
42+
code: '$("div")',
3543
errors: [ { message: error, type: 'CallExpression' } ]
3644
},
3745
{
@@ -65,6 +73,21 @@ ruleTester.run( 'no-global-selector', rule, {
6573
{
6674
code: '$(".selector", $(".context"))',
6775
errors: [ { message: error, type: 'CallExpression' } ]
76+
},
77+
{
78+
code: '$("#id>div")',
79+
errors: [ { message: error, type: 'CallExpression' } ],
80+
options: [ { allowIds: true } ]
81+
},
82+
{
83+
code: '$("#id~div")',
84+
errors: [ { message: error, type: 'CallExpression' } ],
85+
options: [ { allowIds: true } ]
86+
},
87+
{
88+
code: '$("#id div")',
89+
errors: [ { message: error, type: 'CallExpression' } ],
90+
options: [ { allowIds: true } ]
6891
}
6992
]
7093
} );

0 commit comments

Comments
 (0)