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

no-undefined doesn't allow undefined as an object property #7964

Closed
YodaDaCoda opened this issue Jan 23, 2017 · 7 comments · Fixed by renovatebot/renovate#111 · May be fixed by iamhunter/teammates#4
Closed

no-undefined doesn't allow undefined as an object property #7964

YodaDaCoda opened this issue Jan 23, 2017 · 7 comments · Fixed by renovatebot/renovate#111 · May be fixed by iamhunter/teammates#4
Assignees
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

@YodaDaCoda
Copy link

Tell us about your environment

  • ESLint Version: v3.14.0
  • Node Version: v6.9.4
  • npm Version: 3.10.10

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

Please show your full configuration:

{
  "parserOptions": {
    "ecmaVersion": 6
  },
  "env": {
    "browser": true
  },
  "globals": {
    "jQuery": false
  },
  "rules": {
    "quote-props": ["error", "consistent-as-needed"],
    "no-undefined": "error"
  }
}

What did you do? Please include the actual source code causing the issue.

'use strict';

var foo = {
  undefined: 'bar',
};

What did you expect to happen?
undefined should be permitted as an unquoted object property.

What actually happened? Please include the actual, raw output from ESLint.
no-undefined produces an error. Quoting undefined (which would otherwise be a workaround) produces an error from quote-props.

Given that undefined is a permissable object property name, I believe no-undefined should be modified to permit this behaviour.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jan 23, 2017
@platinumazure
Copy link
Member

platinumazure commented Jan 23, 2017

Reproduced in the online demo.

I assume all of the following should be valid cases.

'use strict';

var foo = {
  undefined: 'bar',
};

// ES6 destructuring, case 1
var { undefined: bar } = foo;

// Invalid? ES6 destructuring, case 2
var { bar: undefined } = foo;

@platinumazure platinumazure added accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Jan 23, 2017
@platinumazure platinumazure self-assigned this Jan 23, 2017
@ljharb
Copy link
Sponsor Contributor

ljharb commented Jan 23, 2017

They might only be valid in strict mode, with an ecmaVersion > 5 or 6 - but I think there's other rules that can handle that.

var { undefined: bar } = foo; should be valid because it's extracting a property named "undefined" and putting it into a variable named "bar" - but I don't think var { bar: undefined } = foo; should be valid since it's creating a variable named "undefined" from foo.bar.

@platinumazure
Copy link
Member

@ljharb What about in sloppy ES6, can you assign to undefined via assignment pattern then?

@not-an-aardvark
Copy link
Member

undefined is a non-writable property of the global object in ES5 and above, so it can't be globally reassigned in any context (sloppy or strict). The only difference with strict mode is that a runtime error will occur rather than a silent failure.

A local variable called undefined can be created in any context, regardless of sloppy/strict mode.

@platinumazure
Copy link
Member

platinumazure commented Jan 23, 2017

@not-an-aardvark Thanks, that helps a bit. Unfortunately I'm not super familiar with destructuring. Do these cases create/shadow undefined in local scope, or try to write to the undefined in global scope?

// Case 1
var { bar: undefined } = foo;

// Case 2
({ bar: undefined } = foo);

Feel free to edit this post with inline remarks if it's easier. Thanks for your help.

@not-an-aardvark
Copy link
Member

Case 1 creates a local variable undefined which shadows the global variable. Case 2 reassigns the global variable.

The ES5 equivalents of those cases are:

// Case 1
var undefined = foo.bar;

// Case 2
undefined = foo.bar;

@platinumazure
Copy link
Member

Okay. I assume that assigning to undefined should probably be invalid in each case, then, since it's still trying to use undefined as an identifier rather than it coming up as an object key. I'll write a PR with that assumption, let me know either here or in the PR if that's wrong.

@alberto alberto closed this as completed in e228d56 Feb 7, 2017
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 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 Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Projects
None yet
5 participants