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

feat: Adding a new no-ignored-unsubscribe rule. #592

Merged
merged 4 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/hip-tips-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-svelte": minor
---

feat: add new `svelte/no-ignored-unsubscribe` rule.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ These rules relate to better ways of doing things to help you avoid problems:
| [svelte/block-lang](https://sveltejs.github.io/eslint-plugin-svelte/rules/block-lang/) | disallows the use of languages other than those specified in the configuration for the lang attribute of `<script>` and `<style>` blocks. | |
| [svelte/button-has-type](https://sveltejs.github.io/eslint-plugin-svelte/rules/button-has-type/) | disallow usage of button without an explicit type attribute | |
| [svelte/no-at-debug-tags](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-at-debug-tags/) | disallow the use of `{@debug}` | :star: |
| [svelte/no-ignored-unsubscribe](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-ignored-unsubscribe/) | disallow ignoring the unsubscribe method returned by the `subscribe()` on Svelte stores. | |
| [svelte/no-immutable-reactive-statements](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-immutable-reactive-statements/) | disallow reactive statements that don't reference reactive values. | |
| [svelte/no-reactive-functions](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-functions/) | it's not necessary to define functions in reactive statements | :bulb: |
| [svelte/no-reactive-literals](https://sveltejs.github.io/eslint-plugin-svelte/rules/no-reactive-literals/) | don't assign literal values in reactive statements | :bulb: |
Expand Down
1 change: 1 addition & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ These rules relate to better ways of doing things to help you avoid problems:
| [svelte/block-lang](./rules/block-lang.md) | disallows the use of languages other than those specified in the configuration for the lang attribute of `<script>` and `<style>` blocks. | |
| [svelte/button-has-type](./rules/button-has-type.md) | disallow usage of button without an explicit type attribute | |
| [svelte/no-at-debug-tags](./rules/no-at-debug-tags.md) | disallow the use of `{@debug}` | :star: |
| [svelte/no-ignored-unsubscribe](./rules/no-ignored-unsubscribe.md) | disallow ignoring the unsubscribe method returned by the `subscribe()` on Svelte stores. | |
| [svelte/no-immutable-reactive-statements](./rules/no-immutable-reactive-statements.md) | disallow reactive statements that don't reference reactive values. | |
| [svelte/no-reactive-functions](./rules/no-reactive-functions.md) | it's not necessary to define functions in reactive statements | :bulb: |
| [svelte/no-reactive-literals](./rules/no-reactive-literals.md) | don't assign literal values in reactive statements | :bulb: |
Expand Down
48 changes: 48 additions & 0 deletions docs/rules/no-ignored-unsubscribe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
---
pageClass: 'rule-details'
sidebarDepth: 0
title: 'svelte/no-ignored-unsubscribe'
description: 'disallow ignoring the unsubscribe method returned by the `subscribe()` on Svelte stores.'
---

# svelte/no-ignored-unsubscribe

> disallow ignoring the unsubscribe method returned by the `subscribe()` on Svelte stores.

- :exclamation: <badge text="This rule has not been released yet." vertical="middle" type="error"> **_This rule has not been released yet._** </badge>

## :book: Rule Details

This rule fails if an "unsubscriber" returned by call to `subscribe()` is neither assigned to a variable or property or passed to a function.

One should always unsubscribe from a store when it is no longer needed. Otherwise, the subscription will remain active and constitute a **memory leak**.
This rule helps to find such cases by ensuring that the unsubscriber (the return value from the store's `subscribe` method) is not ignored.

<ESLintCodeBlock>

<!--eslint-skip-->

```svelte
<script>
/* eslint svelte/no-ignored-unsubscribe: "error" */

import myStore from './my-stores';

/* ✓ GOOD */
const unsubscribe = myStore.subscribe(() => {});

/* ✗ BAD */
myStore.subscribe(() => {});
</script>
```

</ESLintCodeBlock>

## :wrench: Options

Nothing.

## :mag: Implementation

- [Rule source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/src/rules/no-ignored-unsubscribe.ts)
- [Test source](https://github.com/sveltejs/eslint-plugin-svelte/blob/main/tests/src/rules/no-ignored-unsubscribe.ts)
32 changes: 32 additions & 0 deletions src/rules/no-ignored-unsubscribe.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import type { TSESTree } from '@typescript-eslint/types';
import { createRule } from '../utils';

export default createRule('no-ignored-unsubscribe', {
meta: {
docs: {
description:
'disallow ignoring the unsubscribe method returned by the `subscribe()` on Svelte stores.',
category: 'Best Practices',
recommended: false
},
fixable: undefined,
hasSuggestions: false,
messages: {
forbidden: 'Ignoring returned value of the subscribe method is forbidden.'
},
schema: [],
type: 'problem'
},
create: (context) => {
return {
"ExpressionStatement > CallExpression > MemberExpression.callee[property.name='subscribe']": (
node: TSESTree.MemberExpression
) => {
context.report({
messageId: 'forbidden',
node: node.property
});
}
};
}
});
2 changes: 2 additions & 0 deletions src/utils/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import noDupeUseDirectives from '../rules/no-dupe-use-directives';
import noDynamicSlotName from '../rules/no-dynamic-slot-name';
import noExportLoadInSvelteModuleInKitPages from '../rules/no-export-load-in-svelte-module-in-kit-pages';
import noExtraReactiveCurlies from '../rules/no-extra-reactive-curlies';
import noIgnoredUnsubscribe from '../rules/no-ignored-unsubscribe';
import noImmutableReactiveStatements from '../rules/no-immutable-reactive-statements';
import noInnerDeclarations from '../rules/no-inner-declarations';
import noNotFunctionHandler from '../rules/no-not-function-handler';
Expand Down Expand Up @@ -88,6 +89,7 @@ export const rules = [
noDynamicSlotName,
noExportLoadInSvelteModuleInKitPages,
noExtraReactiveCurlies,
noIgnoredUnsubscribe,
noImmutableReactiveStatements,
noInnerDeclarations,
noNotFunctionHandler,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- message: Ignoring returned value of the subscribe method is forbidden.
line: 6
column: 6
suggestions: null
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<script>
import { writable } from 'svelte/store';

const foo = writable(0);

foo.subscribe(() => {
console.log('foo changed');
});
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<script>
import { writable } from 'svelte/store';

const foo = writable(0);

const unsubscribe = foo.subscribe(() => {
console.log('foo changed');
});
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<script>
import { writable } from 'svelte/store';

const foo = writable(0);

const unsubscribers = [];
unsubscribers.push(
foo.subscribe(() => {
console.log('foo changed');
})
);
</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<script>
a(foo.subscribe);
</script>
12 changes: 12 additions & 0 deletions tests/src/rules/no-ignored-unsubscribe.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { RuleTester } from 'eslint';
import rule from '../../../src/rules/no-ignored-unsubscribe';
import { loadTestCases } from '../../utils/utils';

const tester = new RuleTester({
parserOptions: {
ecmaVersion: 2020,
sourceType: 'module'
}
});

tester.run('no-ignored-unsubscribe', rule as any, loadTestCases('no-ignored-unsubscribe'));