8000 New rule: no-useless-empty-export · Issue #2757 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

New rule: no-useless-empty-export #2757

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.

Sign up for GitHub

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

Jump to bottom
Closed
remcohaszing opened this issue Nov 12, 2020 · 6 comments · Fixed by #4380
Closed

New rule: no-useless-empty-export #2757

remcohaszing opened this issue Nov 12, 2020 · 6 comments · Fixed by #4380
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new plugin rule New rule request for eslint-plugin package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@remcohaszing
Copy link
Contributor
remcohaszing commented Nov 12, 2020 •
edited
Loading

In TypeScript, modules are TypeScript files that contain an import or export statement. If a file is not a module, it will be ignored by TypeScript. Sometimes a file doesn’t contain imports or exports for legitimate reasons. In this case, an empty export must be added. I.e. to augment an interface:

export {};

declare module '@appsemble/sdk' {
  interface Parameters {
    interval: number;
  }

  interface EventEmitters {
    interval: never;
  }
}

As soon as the file is modified to contain actual imports or exports, the empty export can be removed.

I’d like to propose a rule that reports empty exports if the file contains other imports / exports. This can be autofixable by removing the empty export. I’m willing to implement this.
@remcohaszing remcohaszing added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Nov 12, 2020
@remcohaszing
Copy link
Contributor Author

Perhaps even an option could be added to make an export required if no imports are present. Maybe this would be even better.

rules:
  @typescript-eslint/valid-module:
    - error
      # Removes an empty export if there are other imports or exports. Autofix removes the empty export.
    - allowUselessExport: false
      # Requires an empty export if there are no other imports or exports. Autofix adds an empty export to the end of the file.
      requireModuleSyntax: true

@bradzacher
Copy link
Member

If a file is not a module, it will be ignored by TypeScript

Could you explain what you mean by this?
This hasn't been my experience with TypeScript.

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for team members to take a look labels Nov 13, 2020
@remcohaszing
Copy link
Contributor Author

From the TypeScript Handbook Modules page:

In TypeScript, just as in ECMAScript 2015, any file containing a top-level import or export is considered a module. Conversely, a file without any top-level import or export declarations is treated as a script whose contents are available in the global scope (and therefore to modules as well).

I don’t know the exact details of what this means. I do know a specific situation how it impacts my project.

Clone and setup Appsemble and open it with a TypeScript editor:

git clone https://gitlab.com/appsemble/appsemble.git
cd appsemble
yarn
code .

Now open blocks/timer/src/index.ts and blocks/timer/block.ts

code blocks/timer/src/index.ts
code blocks/timer/block.ts

Everything should be fine. block.ts contains an export {} statement. If this is removed, the file is no longer considered to be a module by TypeScript. An error will appear in src/index.ts.

After reading this article I just googled I believe scripts can’t declare modules.

Adding any import or export back to the file, will make TypeScript stop complaining. The only logical thing to do is add an empty export. However, this statement is useless as soon as another import or export is added.

@bradzacher
Copy link
Member

I'm personally of the opinion that you should never declare in anything except for .d.ts files (aside from a few edge cases).

Doubly true for declaring modules.

importing a random file to declare a module so that you can import another file seems like a really bad pattern?

OTOH if you declare a .d.ts file and put it in the right place, then it becomes an ambient module declaration file, and isn't required to be imported.

@remcohaszing
Copy link
Contributor Author

Declaring a module is just an example I ran into. The actual issue is about TypeScript behaving differently for modules and scripts.

For example the following exports an interface:

export interface Foo {}

However, the following declares a global interface:

interface Foo {}

This ESLint rule would avoid the (probably accidental) global interface declaration by autofixing this to:

interface Foo {}

export {};

Now the developer using a consuming module will be reminded they forgot to export and import Foo. The developer should manually change it to the following:

export interface Foo {}

export {};

Now the ESLint rule should complain the export {} statement is useless and the autofix should remove it.

@bradzacher bradzacher added enhancement: new plugin rule New rule request for eslint-plugin and removed awaiting response Issues waiting for a reply from the OP or another party labels Jul 6, 2021
@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Dec 31, 2021
Merged
3 tasks
@IanVS
Copy link
IanVS commented Apr 28, 2022

Note, TypeScript 4.7 is going to allow more control over detection of modules vs scripts: https://devblogs.microsoft.com/typescript/announcing-typescript-4-7-beta/#control-over-module-detection

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new plugin rule New rule request for eslint-plugin package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(eslint-plugin): add no-useless-empty-export rule JoshuaKGoldberg/typescript-eslint
4 participants
@remcohaszing @JoshuaKGoldberg @IanVS @bradzacher
0