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

no-use-state-initializer-functions rule #138

Merged
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
2 changes: 2 additions & 0 deletions eslint-plugin-expensify/CONST.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,7 @@ module.exports = {
PREFER_TYPE_FEST_VALUE_OF: 'Prefer using `ValueOf` from `type-fest` to extract the type of the properties of an object.',
PREFER_AT: 'Prefer using the `.at()` method for array element access.',
PREFER_SHOULD_USE_NARROW_LAYOUT_INSTEAD_OF_IS_SMALL_SCREEN_WIDTH: 'Prefer using `shouldUseNarrowLayout` instead of `isSmallScreenWidth` from `useResponsiveLayout`.',
NO_USE_STATE_INITIALIZER_CALL_FUNCTION:
'Avoid calling a function directly in the useState initializer. Use an initializer function instead (a callback).',
},
};
84 changes: 84 additions & 0 deletions eslint-plugin-expensify/no-use-state-initializer-functions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
const message = require('./CONST').MESSAGE.NO_USE_STATE_INITIALIZER_CALL_FUNCTION;

module.exports = {
meta: {
type: 'problem', // This is a potential performance issue
docs: {
description:
'Disallow direct function calls in useState initializer',
category: 'Best Practices',
recommended: false,
},
schema: [], // No options for this rule
messages: {
noDirectCall: message,
},
},
create(context) {
return {
CallExpression(node) {
// Return early if the function being called is not `useState`
if (
node.callee.type !== 'Identifier'
|| node.callee.name !== 'useState'
|| node.arguments.length === 0
) {
return;
}

const firstArg = node.arguments[0];

// Return early if the first argument is not a function call, member expression with a function call, or conditional expression with function calls
if (
firstArg.type !== 'CallExpression'
&& !(firstArg.type === 'MemberExpression' && firstArg.object.type === 'CallExpression')
&& !(firstArg.type === 'ConditionalExpression'
&& (firstArg.consequent.type === 'CallExpression' || firstArg.alternate.type === 'CallExpression'))
) {
return;
}

context.report({
node: firstArg,
messageId: 'noDirectCall',
});
},

// Handle cases where the initializer is passed as a function reference
VariableDeclarator(node) {
if (
!node.init
|| node.init.type !== 'CallExpression'
|| node.init.callee.name !== 'useState'
|| node.init.arguments.length === 0
) {
return;
}

const firstArg = node.init.arguments[0];

// Return early if the first argument is a function reference (valid case)
if (
firstArg.type === 'Identifier' // e.g., `getChatTabBrickRoad`
|| (firstArg.type === 'ArrowFunctionExpression'
&& firstArg.body.type !== 'CallExpression') // e.g., `() => getChatTabBrickRoad`
) {
return; // Valid case, do nothing
}

// If it's a direct function call, member expression with a function call, or conditional expression with function calls, report it
if (
firstArg.type === 'CallExpression'
|| (firstArg.type === 'MemberExpression' && firstArg.object.type === 'CallExpression')
|| (firstArg.type === 'ConditionalExpression'
&& (firstArg.consequent.type === 'CallExpression' || firstArg.alternate.type === 'CallExpression'))
) {
context.report({
node: firstArg,
messageId: 'noDirectCall',
});
}
},
};
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
const RuleTester = require('eslint').RuleTester;
const rule = require('../no-use-state-initializer-functions');
const message = require('../CONST').MESSAGE.NO_USE_STATE_INITIALIZER_CALL_FUNCTION;

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

ruleTester.run('no-use-state-initializer-functions', rule, {
valid: [
{
// Calling a callback should be valid
code: `
useState(() => testFunc());
`,
},
{
// Calling a callback should be valid
code: `
useState(() => testFunc().value);
`,
},
{
// Calling a callback should be valid
code: `
useState(condition ? testFunc : testFunc);
`,
},
{
// Calling a callback should be valid
code: `
useState(condition ? () => testFunc() : () => testFunc());
`,
},
],
invalid: [
{
// Calling a function should be invalid
code: `
useState(testFunc());
`,
errors: [
{
message,
},
],
},
{
// Calling a function should be invalid
code: `
useState(testFunc().value);
`,
errors: [
{
message,
},
],
},
{
// Calling a function should be invalid
code: `
useState(condition ? testFunc() : testFunc());
`,
errors: [
{
message,
},
],
},
{
// Calling a function should be invalid
code: `
useState(condition ? (() => testFunc())() : (() => testFunc())());
`,
errors: [
{
message,
},
],
},
],
});
1 change: 1 addition & 0 deletions rules/expensify.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ module.exports = {
}],
}],
'rulesdir/prefer-shouldUseNarrowLayout-instead-of-isSmallScreenWidth': 'warn',
'rulesdir/no-use-state-initializer-functions': 'error',
},
};

Loading