6480 Validation: Typescript Code Smell

Clean Code Report False-positive / False-negative...
1

Template for a good new topic, formatted with Markdown:
We are using Azure DevOps and are developing a NextJS Project using Typescript.

We have the Typescript rule 6480 enabled, that states:

image
image981×578 48.5 KB

The problem is, that I basically the compliant version of the example given in the rule, but the code smell does not go away.
Here is the Code Snippet:

image
image958×64 5 KB

image

The rule was introduced on Dec 07 2022. Maybe there is bug or something for the validation of this rule?
I am not quite sure, but I would like to get some help.

Thank you in advance.

Kind regards
Felix

2 Likes
4

The fix is to switch

async function doLogin() {
    return login({ user: "test" });
}

to

  const doLogin = useCallback(async function doLogin() {
    return login({ user: "test" });
  }, [])

Though useCallback can be used to avoid renders, it has a side effect, it memoizes results based on the input. Now that may be what you’d want for functions that will receive the same value and you want it computed the same way, but another alternative that also satisfies the rule is

  const doLogin = useMemo(() => async function doLogin() {
    return login({ user: "test" });
  }, [])
5

@felixstahmer I do not understand why the issue is raised from the code sample you provided, could you please share the screenshot of the issue from SonarQube and/or share the reproducible code snippet which triggers the issue?

6

@saberduck if you’re talking about the OP his screenshot does show why.

He has a onClick which is taking goToPreviousStep which is a standalone function. Since it’s a standalone function it should trigger the violation.

For him to fix it he just needs to wrap it

const goToPreviousStep = useCallback(function goToPreviousStep() {
  setActiveStep(activeStep - 1, []);
}

It’s a tedious fix but it is a valid violation.

Side note

Actually I have to correct myself earlier. I was mistaken that it memoizes the result. Looking more closely at Hooks API Reference – React (reactjs.org) it states it memoizes the callback which means you don’t need to do the useMemo(()=>()=>{...}, [])

Which BTW may be a good violation to track

Something like

useMemo(()=> () => {...}, []);

should be written as

useCallback(() => {...}, []);
7

We are going to improve the documentation of the rule to better explain non-compliant code and how to fix it. Meanwhile we will remove it from the default profile.

9

Why remove it? Isn’t it a valid violation?

10

Hi @trajano,

There has been some pushback from users, so we take that things might not be as black and white as we initially though.

By disabling it by default, we give us some time to better understand user’s expectations without alienating everyone.

Most probably, we will need to improve the rule before it receives greater adoption.

Of course, you can still use the rule by enabling it manually.

Best,
Gabriel

1 Like
11

Here’s the Github issue for future cross-reference: