sindresorhus / eslint-plugin-unicorn

More than 100 powerful ESLint rules
MIT License
4.29k stars 368 forks source link

Rule proposal: isolated-functions #2214

Open mmkal opened 1 year ago

mmkal commented 1 year ago

Description

It'd be nice to have a rule that makes sure certain functions are free of external dependencies. For example, this won't work:

import makeSynchronous from 'make-synchronous'

export const fetchSync = () => {
  const url = 'https://example.com'
  const getText = makeSynchronous(async () => {
    const res = await fetch(url)
    return res.text()
  })
  console.log(getText())
}

(from make-synchronous docs: "The given function is executed in a subprocess, so you cannot use any variables/imports from outside the scope of the function. You can pass in arguments to the function. To import dependencies, use await import(…) in the function body.")

There are other scenarios where functions should/must avoid using variables from outside their scopes, like server actions (maybe?), or if using myFn.toString().

I don't know exactly what the conditions for banning scope-external variables should be, but maybe makeSynchronous(...)-style calling would be a good start, given it's part of the Sindresorhus Cinematic Universe. Config could look similar to no-restricted-syntax:

'restrict-function-dependencies': ['error', [{
  selector: 'CallExpression[name=makeSynchronous]',
]],

Fail

import makeSynchronous from 'make-synchronous'

export const fetchSync = () => {
  const url = 'https://example.com'
  const getText = makeSynchronous(async () => {
    const res = await fetch(url)
    return res.text()
  })
  console.log(getText())
}

Pass

import makeSynchronous from 'make-synchronous'

export const fetchSync = () => {
  const getText = makeSynchronous(async () => {
    const url = 'https://example.com'
    const res = await fetch(url)
    return res.text()
  })
  console.log(getText())
}
import makeSynchronous from 'make-synchronous'

export const fetchSync = () => {
  const getText = makeSynchronous(async (url) => {
    const res = await fetch(url)
    return res.text()
  })
  console.log(getText('https://example.com'))
}

Additional Info

The devil would be in the details here, and it'd probably be impossible to completely get rid of false positives or false negatives but it could still be useful to (try to) remind people when they can't use normal function scoping.

I had a look for similar ideas in the docs/issues but maybe I missed something? Does this exist?

sindresorhus commented 1 year ago

I like it.


Config could look similar to no-restricted-syntax:

I would base it on the package name instead. makeSynchronous could potentially be a user-defined function that behaves differently.


I think some kind of comment annotation would be useful just to enforce it yourself.

I would call it isolated or stateless. Leaning towards the former.

import makeSynchronous from 'make-synchronous';

export const fetchSync = () => {
  // @isolated
  const getText = makeSynchronous(async () => {
    const url = 'https://example.com'
    const res = await fetch(url)
    return res.text()
  })
  console.log(getText())
}
sindresorhus commented 1 year ago

Related TypeScript issues:

mmkal commented 1 year ago

How about similar options to template-indent

The rule accepts lists of tags, functions, selectors and comments to match template literals. tags are tagged template literal identifiers, functions are names of utility functions like stripIndent, selectors can be any ESLint selector, and comments are / block-commented / strings.

sindresorhus commented 1 year ago

Yeah, I think it could support an object with: modules, functions, selectors.

JounQin commented 10 months ago

This won't work if you rely on TypeScript external helpers like tslib, or esbuild inline helpers.

mmkal commented 7 months ago

I renamed this to isolated-functions - that's probably more appropriate than a no-* type name which makes it seem like any non-isolated functions are automatically bad. People who are very purist about purity might think that but js is not a fully functional language so a name that implies only certain functions must be isolated, based on some config, is better.