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 2 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.
52 changes: 52 additions & 0 deletions docs/rules/no-ignored-unsubscribe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
---
pageClass: 'rule-details'
sidebarDepth: 0
title: 'svelte/no-ignored-unsubscribe'
description: 'disallow ignoring the return value of `subscribe()`'
since: 'v2.34.0'
moufmouf marked this conversation as resolved.
Show resolved Hide resolved
---

# (svelte/no-ignored-unsubscribe)
moufmouf marked this conversation as resolved.
Show resolved Hide resolved

> disallow ignoring the return value of `subscribe()`

## :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.

## :rocket: Version

This rule was introduced in eslint-plugin-svelte v2.34.0

## :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)
30 changes: 30 additions & 0 deletions src/rules/no-ignored-unsubscribe.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import type { TSESTree } from '@typescript-eslint/types';
import { createRule } from "../utils";

export default createRule("no-ignored-unsubscribe", {
meta: {
docs: {
description: "Forbids ignoring the unsubscribe method returned by the `subscribe()` os Svelte stores.",
moufmouf marked this conversation as resolved.
Show resolved Hide resolved
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[property.name='subscribe']":
moufmouf marked this conversation as resolved.
Show resolved Hide resolved
(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 @@ -61,6 +61,7 @@ import system from '../rules/system';
import validCompile from '../rules/valid-compile';
import validEachKey from '../rules/valid-each-key';
import validPropNamesInKitPages from '../rules/valid-prop-names-in-kit-pages';
import noIgnoredUnsubscribe from "../rules/no-ignored-unsubscribe";

export const rules = [
typescriptEslintNoUnnecessaryCondition,
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: 9
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,10 @@
<script>
import {writable} from "svelte/store";

const foo = writable(0);

const unsubscribers = [];
unsubscribers.push(foo.subscribe(() => {
console.log('foo changed');
}));
</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'));
Loading