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-restricted-imports doesn't handle re-export #9678

Closed
asapach opened this issue Dec 2, 2017 · 11 comments
Closed

no-restricted-imports doesn't handle re-export #9678

asapach opened this issue Dec 2, 2017 · 11 comments
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 Hacktoberfest Recommended issue for those participating in Hacktoberfest https://hacktoberfest.digitalocean.com/ rule Relates to ESLint's core rules

Comments

@asapach
Copy link

asapach commented Dec 2, 2017

  • ESLint Version: 4.12.1
{
    "parserOptions": {
        "ecmaVersion": 6,
        "sourceType": "module"
    },
    "rules": {
        "no-restricted-imports": ["error", "fs"]
    }
}
import fs from 'fs';
export * from 'fs';
export {default as fs} from 'fs';
export {readFile} from 'fs';

What did you expect to happen?
All the statements including re-exports should be reported as errors.

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

  1:1  error  'fs' import is restricted from being used  no-restricted-imports

Only import is reported; re-exports are ignored.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Dec 2, 2017
@ilyavolodin
Copy link
Member

The rule is supposed to only check import statements, so I'm not sure why would it report an export.

@j-f1
Copy link
Contributor

j-f1 commented Dec 2, 2017

This is because the export ... from 'module' is not yet part of JavaScript, and ESLint doesn’t support features that are still proposals. This would be a good rule suggestion for eslint-plugin-babel, though. This is incorrect; see below.

@asapach
Copy link
Author

asapach commented Dec 2, 2017

@ilyavolodin, Because a re-export is essentially an import + export. So export {readFile} from 'fs' is equivalent to:

import {readFile} from 'fs';
export {readFile};

By ignoring re-exports you are leaving an escape hatch for no-restricted-imports, so people can re-export forbidden modules and import them elsewhere.

@j-f1 I'm talking about ES2015 syntax; future proposals are out of scope.

@j-f1
Copy link
Contributor

j-f1 commented Dec 2, 2017

@asapach I’m trying to say that everything but the first line in your initial code sample is not part of ES2015, ES2016, or ES2017. It’s currently a stage 1 proposal (export v from "mod"; statements). Therefore, it’s out-of-scope for ESLint.

ES2015 only supports this:

import foo from 'bar'
import { foo, bar as baz } from 'quux'

export const /* or let/var */ foo = 2
export {
  foo as bar,
  baz
}
export default foo

However, I would be interested in seeing this checked by eslint-plugin-babel.

This is incorrect; see below.

@asapach
Copy link
Author

asapach commented Dec 2, 2017

@j-f1 the proposal is specifically about a more convenient syntax for re-exporting default and has nothing to do with this issue.

@j-f1
Copy link
Contributor

j-f1 commented Dec 2, 2017

🤦‍♂️ sorry. Thanks for sharing that with me! I must’ve misread the proposals I linked to. I’ll take a look at this and see if I can fix it.

Notes to self: rule source, copy-paste[?] ImportDeclaration to "ExportAllDeclaration, ExportNamedDeclaration". ExportAll -> isRestrictedForEverythingImported, ExportNamed -> isRestrictedPath

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

@j-f1 There's no rush, but have you gotten a chance to take a look at this?

@j-f1
Copy link
Contributor

j-f1 commented Jan 7, 2018

I have not gotten a chance to look at this yet.

@kaicataldo kaicataldo added the Hacktoberfest Recommended issue for those participating in Hacktoberfest https://hacktoberfest.digitalocean.com/ label Oct 15, 2018
@nzakas nzakas self-assigned this Nov 7, 2018
@nzakas
Copy link
Member

nzakas commented Nov 7, 2018

I'll look at this.

@ljharb
Copy link
Sponsor Contributor

ljharb commented Nov 7, 2018

Linking to #11064.

@btmills btmills closed this as completed in 79a2797 Nov 9, 2018
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators May 9, 2019
@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 May 9, 2019
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 Hacktoberfest Recommended issue for those participating in Hacktoberfest https://hacktoberfest.digitalocean.com/ rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants