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

parent in load functions receive @typescript-eslint/unbound-method error #12622

Open
rakuzen25 opened this issue Aug 28, 2024 · 5 comments · May be fixed by #12915 or #12955
Open

parent in load functions receive @typescript-eslint/unbound-method error #12622

rakuzen25 opened this issue Aug 28, 2024 · 5 comments · May be fixed by #12915 or #12955

Comments

@rakuzen25
Copy link

rakuzen25 commented Aug 28, 2024

Describe the bug

My +page.ts looks like something like this:

export const load: PageLoad = async ({ fetch, parent }) => {
    const { uid } = (await parent()).session?.user ?? {};
    if (!uid) {
        throw new TypeError("User ID not found");
    }

    // Do something
};

However, eslint seems not very happy about the parent function reference and throws

Avoid referencing unbound methods which may cause unintentional scoping of `this`.
If your function does not access `this`, you can annotate it with `this: void`, or consider using an arrow function instead. eslint(@typescript-eslint/unbound-method)

Docs on unbound-method

Reproduction

MRE on Stackblitz

Run pnpm eslint ./src to see the errors.

Logs

/home/projects/stackblitz-starters-1y7qpk/src/routes/sum/+layout.ts
  3:42  error  Avoid referencing unbound methods which may cause unintentional scoping of `this`.
If your function does not access `this`, you can annotate it with `this: void`, or consider using an arrow function instead  @typescript-eslint/unbound-method

/home/projects/stackblitz-starters-1y7qpk/src/routes/sum/+page.ts
  3:40  error  Avoid referencing unbound methods which may cause unintentional scoping of `this`.
If your function does not access `this`, you can annotate it with `this: void`, or consider using an arrow function instead  @typescript-eslint/unbound-method

✖ 2 problems (2 errors, 0 warnings)

System Info

System:
  OS: Linux 5.0 undefined
  CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  Memory: 0 Bytes / 0 Bytes
  Shell: 1.0 - /bin/jsh
Binaries:
  Node: 18.20.3 - /usr/local/bin/node
  Yarn: 1.22.19 - /usr/local/bin/yarn
  npm: 10.2.3 - /usr/local/bin/npm
  pnpm: 8.15.6 - /usr/local/bin/pnpm
npmPackages:
  @sveltejs/adapter-auto: ^3.0.0 => 3.2.4 
  @sveltejs/kit: ^2.0.0 => 2.5.25 
  @sveltejs/vite-plugin-svelte: ^3.0.0 => 3.1.2 
  svelte: ^4.2.7 => 4.2.19 
  vite: ^5.0.3 => 5.4.2

Severity

annoyance

Additional Information

Patrick in the server mentioned that perhaps

/**
* `await parent()` returns data from parent `+layout.js` `load` functions.
* Implicitly, a missing `+layout.js` is treated as a `({ data }) => data` function, meaning that it will return and forward data from parent `+layout.server.js` files.
*
* Be careful not to introduce accidental waterfalls when using `await parent()`. If for example you only want to merge parent data into the returned output, call it _after_ fetching your other data.
*/
parent(): Promise<ParentData>;

and

/**
* `await parent()` returns data from parent `+layout.server.js` `load` functions.
*
* Be careful not to introduce accidental waterfalls when using `await parent()`. If for example you only want to merge parent data into the returned output, call it _after_ fetching your other data.
*/
parent(): Promise<ParentData>;

needs to be changed to parent(this: void): Promise<ParentData>; or parent: () => Promise<ParentData>;, but it seems like this file is auto-generated…

@Stadly
Copy link

Stadly commented Oct 22, 2024

This also applies to other functions, such as resolve in Handle. I created an issue in the svelte repo for it, but it obviously belongs here in the kit repo.

Just posting the relevant details here, as it is the exact same issue as the OP is describing.

Describe the bug

When destructuring a function out of a function argument, for example like this:

// `resolve` is a function that is destructured out of the function argument
async function handle({ event, resolve }) {
  // ...
}

ESLint yells at me:

error  Avoid referencing unbound methods which may cause unintentional scoping of `this`.
If your function does not access `this`, you can annotate it with `this: void`, or consider using an arrow function instead  @typescript-eslint/unbound-method

Destructuing the function argument is very common, and is the way it's done in both the Docs and Tutorial for the handle hook.

I don't think resolve uses this, so I think the issue would be solved by declaring Handle like this (using arrow function for resolve):

export type Handle = (input: {
  event: RequestEvent;
  resolve: (event: RequestEvent, opts?: ResolveOptions) => MaybePromise<Response>;
}) => MaybePromise<Response>;

or this (annotating resolve with this: void):

export type Handle = (input: {
  event: RequestEvent;
  resolve(this: void, event: RequestEvent, opts?: ResolveOptions): MaybePromise<Response>;
}) => MaybePromise<Response>;

instead of this (resolve is a regular function):

export type Handle = (input: {
  event: RequestEvent;
  resolve(event: RequestEvent, opts?: ResolveOptions): MaybePromise<Response>;
}) => MaybePromise<Response>;

Note: The same issue applies to some other functions. At least parent in LayoutLoad and PageLoad comes to mind.

Reproduction

Set up SvelteKit with ESLint and @typescript-eslint/recommended-type-checked.

Set up a handle hook:

// src/hooks.server.js
import type { Handle } from "@sveltejs/kit";

export const handle: Handle = async ({ event, resolve }) => {
  return await resolve(event);
}

@eltigerchino eltigerchino added bug Something isn't working types / typescript labels Oct 22, 2024
@eltigerchino
Copy link
Member

eltigerchino commented Oct 22, 2024

This is strange because those functions are arrow functions although it reports otherwise. Adding the this: void annotation in public.d.ts and turning it into an arrow function doesn't help either.

@eltigerchino eltigerchino removed the bug Something isn't working label Oct 22, 2024
@Stadly
Copy link

Stadly commented Oct 22, 2024

This is strange because those functions are arrow functions although it reports otherwise. Adding the this: void annotation in public.d.ts and turning it into an arrow function doesn't help either.

Strange indeed! I also did a little test making changes to public.d.ts inside the node_modules folder of my project without getting it to work 🤔

@Stadly
Copy link

Stadly commented Oct 22, 2024

Any idea who might be able to look into this, @eltigerchino?

@eltigerchino
Copy link
Member

Any idea who might be able to look into this, @eltigerchino?

Not at the moment. The team is still busy with the post-launch of Svelte 5.

aloisklink added a commit to aloisklink/sveltekit that referenced this issue Oct 30, 2024
Enable the [ESLint `@typescript-eslint/unbound-method-error` rule][1]
and fix/ignore all the errors it throws at us.

Basically, this rule warns us from unbinding a function (e.g. doing
`const {x} = {x() { /* etc */}}` that doesn't explicitly have `this:
void`.

I've replaced these types with arrow functions when possible, to avoid
users of the SvelteKit library from facing the same ESLint issues.

I've also manually fixed some types in
`packages/kit/src/exports/public.d.ts` that weren't caught by the
`@typescript-eslint/unbound-method-error` rule, since they're only
destructured in `packages/kit/test/apps`, which is ignored by our ESLint
config. However, this manual change should
fix sveltejs#12622.

[1]: https://typescript-eslint.io/rules/unbound-method/
Fix: sveltejs#12622
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants