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

object-curly-newline counts default export in minProperties #10101

Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules

Comments

@kadamwhite
Copy link

kadamwhite commented Mar 19, 2018

Tell us about your environment

Ubuntu 17.10

  • ESLint Version: 4.19.0
  • Node Version: 9.6.1
  • npm Version: 5.7.1

(Also validated on OSX, Node 7.10.0, npm 4.2.0)

What parser (default, Babel-ESLint, etc.) are you using?

Default, I believe(?), with sourceType: module. Full configuration:

Configuration

.eslintrc

{
	"root": true,
	"env": { "es6": true },
	"extends": [
		"eslint:recommended"
	],
	"parserOptions": {
		"sourceType": "module"
	},
	"rules": {
		"object-curly-newline": [ "error", {
			"minProperties": 2,
			"consistent": true
		} ]
	}
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

This gist contains a reduced test case with the package.json, .eslintrc and test.js as described here; downloading that gist, running npm install and then npm test replicates the issue on every environment I can access.

test.js

import DefaultExport, { NamedExport } from 'some-module';
import { NamedExportWithoutDefaultImport } from 'some-other-module';

export default {
  DefaultExport,
  NamedExport,
  NamedExportWithoutDefaultImport,
};

package.json

{
  "name": "test-object-curly-newline",
  "scripts": {
    "test": "eslint ./test.js"
  },
  "devDependencies": {
    "eslint": "^4.19.0"
  }
}
npm test # runs `eslint ./test.js`

What did you expect to happen?

I expected no object-curly-newline issue to be reported for either of these imports, as they both have only a single named export within curly brackets:

import DefaultExport, { NamedExport } from 'some-module';
import { NamedExportWithoutDefaultImport } from 'some-other-module';

however the default export on the first line is being counted as an object property, meaning that the rule matches the minProperties and I get errors for each curly bracket on Line 1, but not Line 2

What actually happened? Please include the actual, raw output from ESLint.

$ npm test

> test-object-curly-newline@ test /hm/gh/coding-standards/packages/eslint-config-humanmade/test
> eslint ./test.js


/hm/gh/coding-standards/packages/eslint-config-humanmade/test/test.js
  1:23  error  Expected a line break after this opening brace   object-curly-newline
  1:37  error  Expected a line break before this closing brace  object-curly-newline

✖ 2 problems (2 errors, 0 warnings)
  2 errors, 0 warnings potentially fixable with the `--fix` option.

npm ERR! Test failed.  See above for more details.
@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Mar 19, 2018
@platinumazure platinumazure added bug ESLint is working incorrectly rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Mar 19, 2018
@platinumazure
Copy link
Member

I've confirmed this in our online demo. Setting minProperties: 3 makes the example pass linting, so the proposed explanation is very likely to be correct.

I'll work on this.

@platinumazure platinumazure added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Mar 19, 2018
platinumazure added a commit that referenced this issue Mar 19, 2018
minProperties only tracks ImportSpecifier and ExportSpecifier nodes
This was referenced Mar 22, 2018
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Sep 18, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.