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-use-before-define false positive with binding exports #7858

Closed
Jessidhia opened this issue Jan 5, 2017 · 4 comments
Closed

no-use-before-define false positive with binding exports #7858

Jessidhia opened this issue Jan 5, 2017 · 4 comments
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

@Jessidhia
Copy link

Jessidhia commented Jan 5, 2017

Tell us about your environment

  • ESLint Version: 3.12.2
  • Node Version: 7.2.0
  • npm Version: (yarn) 0.18.1

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

babel-eslint

Please show your full configuration:

// too big, relevant part
module.exports = {
  parser: 'babel-eslint',
  parserOptions: {
    sourceType: 'module'
  },
  rules: {
    'no-use-before-define': ['error', {functions: false, classes: false}]
  }
}

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

export { group as default }
const group = new Set()

What did you expect to happen?

No warnings.

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

   1:10  error  'group' was used before it was defined       no-use-before-define

Binding exports do not read the variable, so they do not cause TDZ errors.

Note that, when circular imports are involved, it is possible to cause a TDZ error by accessing an export whose binding is still in TDZ, but that would happen regardless of the position of the export binding.

Also note that only binding exports should not be errors. export default should error as it is a value*, not a binding:

export default Group // ReferenceError
class Group {}

[*]: it is a binding when exporting FunctionDeclaration or ClassDeclaration, but those are already correctly handled, in the export default case, by the rule with the appropriate options.

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jan 5, 2017
@mysticatea mysticatea added bug ESLint is working incorrectly evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Jan 5, 2017
@mysticatea
Copy link
Member

I confirmed it from the spec. ExportDeclarations seem to do nothing at runtime if the following cases.
But I want other eyes since I don't have confidence (especially static behaviors).

ExportDeclaration:
  export * FromClause;
  export ExportClause FromClause;
  export ExportClause;

Return NormalCompletion(empty).

https://www.ecma-international.org/ecma-262/7.0/#sec-exports-runtime-semantics-evaluation

@not-an-aardvark
Copy link
Member

@ljharb Do you happen to know the correct behavior for this?

@ljharb
Copy link
Sponsor Contributor

ljharb commented Jan 5, 2017

My initial impression was that the last example should work just fine. However, I think @Kovensky is correct - export default takes a value, not a binding, and since class Group {} hoists a binding under TDZ, export default Group would indeed trigger a reference error. However, export { Group } at the top would not, because that exports the binding, and it's only upon unpacking that binding that the TDZ error would occur.

I can't speak to the spec text since grammar stuff isn't my specialty, but this is now my intuition having thought it through.

@not-an-aardvark not-an-aardvark 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 Jan 19, 2017
@not-an-aardvark
Copy link
Member

Closing this issue since the consensus in the PR was that the current default behavior is correct.

@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
Development

No branches or pull requests

5 participants