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

[TypeScript] Functions which do not use this should be typed as arrow functions #4860

Open
1 of 5 tasks
marlokessler opened this issue Sep 13, 2024 · 1 comment
Open
1 of 5 tasks
Labels
👕 TypeScript TypeScript typings issue

Comments

@marlokessler
Copy link

marlokessler commented Sep 13, 2024

What happened?

Background

In our project, we have the ESLint rule "@typescript-eslint/unbound-method" enabled. This rule warns us, when we use non-arrow functions without their context, because in those functions this might be called, unknowingly from the developer. This happens, e.g. when you destructure an object:

const o = {
  greeting: "Hello World",
  logGreeting() {
    console.log(this.greeting);
  },
};

o.logGreeting(); // Output: _"Hello World"_

const { logGreeting } = o;

logGreeting(); // Output: _undefined_

So to prevent unwrapping functions which are bound to an object, this rule is very helpful.

Issue

However, when destructuring useFieldArray according to the VeeValidate documentation, the same error warning occured.
CleanShot 2024-09-13 at 12 21 52

After a look at the source code, I saw that none of the inner functions accessed context with this. So the code worked (of course). However when looking at the type definition of FieldArrayContext I realized, that the functions are declared as normal functions. It's nothing wrong with the definitions, however, normal function types include the assumption that those functions might access context with this and therefore, destructuring is marked as unsafe.

Solution proposal

Since the functions do not access context, arrow function types can be used too:

interface FieldArrayContext<TValue = unknown> {
    fields: Ref<FieldEntry<TValue>[]>;
    remove: (idx: number) => void;
    replace: (newArray: TValue[]) => void;
    update: (idx: number, value: TValue) => void;
    push: (value: TValue) => void;
    swap: (indexA: number, indexB: number) => void;
    insert: (idx: number, value: TValue) => void;
    prepend: (value: TValue) => void;
    move: (oldIdx: number, newIdx: number) => void;
}

This would represent the code more exact, because the functions are also declared independently from a context. This could also be changed for other type declarations too.

Reproduction steps

  1. Setup a project with VeeValidate, ESLint and @typescript-eslint/eslint-plugin
  2. Make sure the rule @typescript-eslint/unbound-method is enabled.
  3. Destructure the returned object of useFieldArray (e.g. const { fields, remove, move } = useFieldArray<SomeType>("path");)
  4. The linting should show a warning

Version

Vue.js 3.x and vee-validate 4.x

What browsers are you seeing the problem on?

  • Firefox
  • Chrome
  • Safari
  • Microsoft Edge

Relevant log output

No response

Demo link

https://codesandbox.io/p/devbox/friendly-haslett-rqvyc5?workspaceId=88e64305-8239-47e1-bc7e-8e2730ac554d

Code of Conduct

@logaretm
Copy link
Owner

logaretm commented Sep 13, 2024

Interesting, I wasn't aware this would affect someone. Thanks for raising this. Do you think you can PR it?

@logaretm logaretm added the 👕 TypeScript TypeScript typings issue label Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👕 TypeScript TypeScript typings issue
Projects
None yet
Development

No branches or pull requests

2 participants