selaux/eslint-plugin-filenames

version 1.3.0 should have been a breaking change

Closed this issue ยท 7 comments

The changes introduced in the latest (1.3.0) package release should have determined a major version bump as they 'break' the current validation.

Our build now fails as

// SomeContainer.js
import React from 'react';
import {withRouter} from 'react-router-dom';

class SomeContainer extends React.Component {
  // stuff
}

export default withRouter(SomeContainer)

now fails (as withRouter name does not match SomeContainer)

I really don't think #29 was a good idea as it's matching the name of the called function and does not consider, instead, the name of the value returned by that called function (that eslint cannot know as it would need some kind of dynamic code analysis), making this new rule quite annoying.
The rule is not even working properly as it doesn't understand chained function and because of that LUCKILY connect(mapStateToProps, mapDispatchToProps)(SomeContainer) still pass the tests.
It's my opinion that export default someAugmentingFunction(SomeContainer) is semantically clear enough and that should be considered valid by this rule.

Excluding my personal opinion, this rule had to be considered a breaking change, or, alternatively a toggle to disable the function exports name check, would have been introduced and disabled by default.

๐Ÿ‘ 1.3.0 breaks here as well with decorated exports

/Users/craigbeck/dev/premise-frontend-v3/packages/webapps/campaign-webapp/src/form/component/FormsList.react.js
  3:1  error  Filename 'FormsList' must match the exported name 'CSSModules'  filenames/match-exported

We use CSSModules extensively and hav many components decorated like this:

export default CSSModule(FileList, styles);

where this is in FileList.react.js

This is a pretty common pattern and I would expect it to be supported in some way

Yes that was an oversight on my part. A solution that I see would be to make it configurable which methods for export detection are used. In the meanwhile I will publish a version that disables the new feature by default in the next days...

thanks @selaux

1.3.1 is released, please test...

It seem there are not options for all calls to getExportedName

> eslint . --cache --cache-file eslint-cache/eslintcache

Cannot read property '2' of undefined
TypeError: Cannot read property '2' of undefined
    at getNodeName (/Users/craigbeck/dev/premise-frontend-v3/node_modules/eslint-plugin-filenames/lib/common/getExportedName.js:10:16)
    at getExportedName (/Users/craigbeck/dev/premise-frontend-v3/node_modules/eslint-plugin-filenames/lib/common/getExportedName.js:33:20)
    at Program (/Users/craigbeck/dev/premise-frontend-v3/node_modules/eslint-plugin-filenames/lib/rules/match-regex.js:28:39)
    at listeners.(anonymous function).forEach.listener (/Users/craigbeck/dev/premise-frontend-v3/node_modules/eslint/lib/util/safe-emitter.js:47:58)
    at Array.forEach (<anonymous>)
    at Object.emit (/Users/craigbeck/dev/premise-frontend-v3/node_modules/eslint/lib/util/safe-emitter.js:47:38)
    at NodeEventGenerator.applySelector (/Users/craigbeck/dev/premise-frontend-v3/node_modules/eslint/lib/util/node-event-generator.js:251:26)
    at NodeEventGenerator.applySelectors (/Users/craigbeck/dev/premise-frontend-v3/node_modules/eslint/lib/util/node-event-generator.js:280:22)
    at NodeEventGenerator.enterNode (/Users/craigbeck/dev/premise-frontend-v3/node_modules/eslint/lib/util/node-event-generator.js:294:14)
    at CodePathAnalyzer.enterNode (/Users/craigbeck/dev/premise-frontend-v3/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:608:23)

As far as I can tell this is happening in a file w /* eslint-disable */ with an export like

module.exports = {
  process: (src, filename, config, transformOptions) => {
    const parts = filename.split('/');
    /* more code */
  }
}

Crap. I released the next version, that should fix it.

Looks good @selaux

eslint passes now, thanks