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

Error thrown on nth-child with ExperimentalSpreadSyntax #67

Closed
aduth opened this issue May 7, 2018 · 4 comments
Closed

Error thrown on nth-child with ExperimentalSpreadSyntax #67

aduth opened this issue May 7, 2018 · 4 comments

Comments

@aduth
Copy link

aduth commented May 7, 2018

Minimal reproduceable example:

https://astexplorer.net/#/gist/e0fb7d08090a532c3cdce571593671b2/622c0d4fed220a6b8d6655f0f746916aeb8bfb60

The :nth-child selector throws an error if object spread syntax (type of ExperimentalSpreadSyntax) is encountered.

Stack trace:

> eslint .

Cannot read property 'length' of undefined
TypeError: Cannot read property 'length' of undefined
    at nthChild (/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/node_modules/esquery/esquery.js:256:34)
    at matches (/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/node_modules/esquery/esquery.js:158:25)
    at Function.matches (/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/node_modules/esquery/esquery.js:103:25)
    at NodeEventGenerator.applySelector (/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/node_modules/eslint/lib/util/node-event-generator.js:250:21)
    at NodeEventGenerator.applySelectors (/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/node_modules/eslint/lib/util/node-event-generator.js:278:22)
    at NodeEventGenerator.enterNode (/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/node_modules/eslint/lib/util/node-event-generator.js:294:14)
    at CodePathAnalyzer.enterNode (/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:608:23)
    at Traverser.enter [as _enter] (/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/node_modules/eslint/lib/linter.js:1006:32)
    at Traverser._traverse (/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/node_modules/eslint/lib/util/traverser.js:132:14)
    at Traverser._traverse (/Users/andrew/Documents/Code/vvv/www/editor/htdocs/wp-content/plugins/gutenberg/node_modules/eslint/lib/util/traverser.js:147:30)

Throwing line:

for (i = 0, l = keys.length; i < l; ++i) {

Real-world use-case:

I am trying to create a no-restricted-syntax ESLint rule for forbidding all but ArrayExpression as the second argument of Lodash methods which accept path argument:

CallExpression[callee.name=/^(invokeMap|get|has|hasIn|invoke|result|set|setWith|unset|update|updateWith)$/] > :not(ArrayExpression):nth-child(2)

This works in isolated usage:

https://astexplorer.net/#/gist/e0fb7d08090a532c3cdce571593671b2/b70d4572529d872e27f8c3df62c4ce0be43dba88

...but with above error, fails with the object spread syntax.

@michaelficarra
Copy link
Member

The version of estraverse we use does not support ExperimentalSpreadSyntax nodes.

@brettz9
Copy link
Contributor

brettz9 commented May 20, 2020

@michaelficarra , what do you think of providing a method to have the user build an esquery instance with their own traverser and/or VisitorKeys baked in? (There was #105 , but maybe exposing a method would be clearer?)

The current default export could be made to rely on it, but if you were in favor of the idea, it wouldn't need to be a breaking change unless:

  1. You wanted to change to named exports
  2. You wanted to change to avoid bundling any default traverser/VisitorKeys at all

In the case of this particular issue, however, if it is possible for @aduth to use espree, I note that ESLint's default parser now, espree, can handle this syntax (as can the current version of estraverse in esquery), albeit as SpreadElement instead of ExperimentalSpreadSyntax, and requiring parser options at least as high as {ecmaVersion: 2018}.

@brettz9
Copy link
Contributor

brettz9 commented Feb 5, 2021

I believe this can be closed now (if the user supplies their own custom keys).

@michaelficarra
Copy link
Member

Fixed by #112.

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

No branches or pull requests

3 participants