next-drupal icon indicating copy to clipboard operation
next-drupal copied to clipboard

Type mismatch between typings declaration files and implementation of the getMenu method

Open lorenzo-dallamuta opened this issue 1 year ago • 2 comments

Package containing the bug

next (Drupal module)

Describe the bug

I need to extend the getMenu method of the DrupalClient class in a Typescript environment. To do so I need the Typescript generic for getMenu to extend DrupalMenuLinkContent because one of the return values also needs to be used as a parameter of the buildMenuTree method.

Note that in the typings of the class downloaded by pnpm the generic only has a default assignement (for me node_modules/.pnpm/[email protected]_@[email protected]/node_modules/next-drupal/dist/client.d.ts), but the implementation extends the generic from the needed type (at packages/next-drupal/src/get-menu.ts).

Expected behavior

I expect to be able to override the implementation of the getMenu method on the DrupalClient class.

Steps to reproduce:

  1. I extend the DrupalClient class in a Typescript environment
  2. I override the getMenu method with the original implementation typings where the generic extends DrupalMenuLinkContent
  3. I get I type mismatch because in the library typings that come bundled with the library the generic is only set to a default of DrupalMenuLinkContent without extending it

Additional context

I'm not sure if those types are included with the repo (couldn't find them there) or pnpm pulls them from something like definitely typed, but I think the typings of getMenu I see when using the library should coincide with the actual implementation.

lorenzo-dallamuta avatar Feb 13 '24 10:02 lorenzo-dallamuta

FYI, packages/next-drupal/src/get-menu.ts is not the method used in DrupalClient; it's the deprecated code before DrupalClient was introduced in 1.6.

The type definition for the getMenu method is in src/client.ts. With DrupalMenuLinkContent defined in packages/next-drupal/src/types/drupal.ts

JohnAlbin avatar Feb 22 '24 17:02 JohnAlbin

In packages/next-drupal/src/types/drupal.ts I see this snippet:

const items = options.deserialize ? this.deserialize(data) : data const { items: tree } = this.buildMenuTree(items)

where items is typed as any because data is typed as any (and the output of deserialize is unknown). I guess the next drupal codebase isn't getting a typescript error because of how types are inferred with ternary operations involving unknown.

In this case I don't think that relying on catching a runtime error is particularly problematic , but I still don't understand why the generic T isn't required to at least extend DrupalMenuLinkContent.

lorenzo-dallamuta avatar Feb 28 '24 11:02 lorenzo-dallamuta