Skip to content

destructuring-assignment is too greedy. Should only apply within components. #1800

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

Closed
milesj opened this issue May 25, 2018 · 6 comments
Closed

Comments

@milesj
Copy link

milesj commented May 25, 2018

I enabled this rule and immediately ran into issues within files that aren't React related, nor do they look like a component. For example:

I had to rename these to properties instead of props: milesj/shapeshifter@52e33fe#diff-392ca8d88bc7e496ea34c73c8cc8258c

Had to disable this line because it thought context was React related: https://github.com/milesj/boost/blob/master/tests/Routine.test.js#L825

And all of these errors when pulled into another project. None of which is React.

/Users/Miles/Sites/beemo/packages/core/src/Beemo.ts
  188:23  error  Must use destructuring context assignment  react/destructuring-assignment
  221:73  error  Must use destructuring context assignment  react/destructuring-assignment
  244:41  error  Must use destructuring context assignment  react/destructuring-assignment

/Users/Miles/Sites/beemo/packages/core/src/CleanupRoutine.ts
  25:7  error  Must use destructuring context assignment  react/destructuring-assignment

/Users/Miles/Sites/beemo/packages/core/src/configure/CreateConfigRoutine.ts
   49:34  error  Must use destructuring context assignment  react/destructuring-assignment
   55:5   error  Must use destructuring context assignment  react/destructuring-assignment
  124:21  error  Must use destructuring context assignment  react/destructuring-assignment
  124:46  error  Must use destructuring context assignment  react/destructuring-assignment

/Users/Miles/Sites/beemo/packages/core/src/driver/RunCommandRoutine.ts
   43:26  error  Must use destructuring context assignment  react/destructuring-assignment
   73:5   error  Must use destructuring context assignment  react/destructuring-assignment
  214:24  error  Must use destructuring context assignment  react/destructuring-assignment
  215:25  error  Must use destructuring context assignment  react/destructuring-assignment
  241:11  error  Must use destructuring context assignment  react/destructuring-assignment
  268:20  error  Must use destructuring context assignment  react/destructuring-assignment
  269:47  error  Must use destructuring context assignment  react/destructuring-assignment

/Users/Miles/Sites/beemo/packages/core/src/ExecuteScriptRoutine.ts
  25:32  error  Must use destructuring context assignment  react/destructuring-assignment

/Users/Miles/Sites/beemo/packages/core/src/SyncDotfilesRoutine.ts
  36:32  error  Must use destructuring context assignment  react/destructuring-assignment

I see that you're checking for a component, but perhaps TS is conflicting.

@ljharb
Copy link
Member

ljharb commented May 25, 2018

It’s possible that our component detection breaks with the TypeScript parser. Do the errors still occur if the files are converted to regular JS?

@ljharb
Copy link
Member

ljharb commented May 25, 2018

I also see that this only seems to complain about context; so maybe there’s a specific bug around that facet of the rule.

@milesj
Copy link
Author

milesj commented May 25, 2018

I'll try and double check JS only files, will be a bit tough.

@bradennapier
Copy link

bradennapier commented Jun 22, 2018

Every single file in my project is now errors - no react imported or used anywhere close. Seems you guys just look for any variable calls "props" or "context" and expect you're in a react component...

export default (context: $Context) => ({
  extensions: ['.js', '.jsx', '.json'],
  modules: [
    /**
     * Resolve imports throughout the application in this order.
     * This allows us to import various parts of the application
     * easier for better organization.
     *
     * Modules always resolve top-down.
     */
    context.dirs.view(),
    path.join(context.dirs.src(), 'libraries'),
    'shared',
    'node_modules',
  ],
});

image

react/destructuring-assignment

@ljharb
Copy link
Member

ljharb commented Jun 22, 2018

@bradennapier that's a separate issue (and definitely a bug) - can you file that as a separate issue?

@heyask
Copy link

heyask commented Jul 6, 2018

Maybe this temporary solution works. add following line in "rules" of .eslintrc.js

"react/destructuring-assignment": [true, { "extensions": [".jsx"] }],

but It may also occur again with some other eslint-plugin-react rules later, in .js file.
I want to restrict all eslint-plugin-react warning only in .jsx file. but I don't know how to.

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

No branches or pull requests

4 participants