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

Rule proposal: no inline callbacks before other parameters #2234

Open
fregante opened this issue Dec 19, 2023 · 2 comments
Open

Rule proposal: no inline callbacks before other parameters #2234

fregante opened this issue Dec 19, 2023 · 2 comments

Comments

@fregante
Copy link
Collaborator

fregante commented Dec 19, 2023

Description

Initially reported in sindresorhus/memoize#97, it's bad practice to place arguments after functions because functions tend to be inlined and eclipse the parameters that follow them.

This could actually be two rules:

  • don't allow calling foo(() => {}, {something: true})
  • don't allow declaring function foo(cb, options) {}

@sindresorhus doesn't like the latter so I'll focus on the first part 😅

Fail

element.addEventListener('click', () => {
	// long
}, {once: true})
const memoized = memoize(() => {
	// long
}, {
	cache: new WeakMap(),
})

Pass

element.addEventListener('click', () => {
	// long
}) // No follow-up argument
const memoized = memoize(() => {
	// long
}) // No follow-up argument
const handler = () => {
	// long
};
element.addEventListener('click', handler, {once: true})
const raw = () => {
	// long
};
const memoized = memoize(raw, {
	cache: new WeakMap(),
})

{maxStatements: 3}

element.addEventListener('click', () => short(), {once: true})
const memoized = memoize((x) => alert(x), {
	cache: new WeakMap(),
})

Additional info

This might conflict with https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-array-callback-reference.md in some cases (reduce) if maxStatements defaults to 0

const foo = array.reduce(callback, 0);
// ❌ no-array-callback-reference

👇

const foo = array.reduce((a, b) => a + b, 0);
// ❌ no-inline-function-sometimes
@sindresorhus
Copy link
Owner

How about allowing one-line functions?

@fregante
Copy link
Collaborator Author

fregante commented Dec 19, 2023

What about

mem(async url => {
  const r = await fetch(url);
  return r.json();
}, {
  something
};

For the purpose of this rule, this still seems readable. That's why it should probably default to "3 statements" and warn that "the inline function is too long and impacts readability" for anything longer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants