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

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

Open
lorenzo-dallamuta opened this issue Feb 13, 2024 · 2 comments
Labels
area: next-drupal bug Something isn't working

Comments

@lorenzo-dallamuta
Copy link

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/next-drupal@1.6.0_@babel+core@7.23.7/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 lorenzo-dallamuta added bug Something isn't working triage A new issue that needs triage labels Feb 13, 2024
@JohnAlbin
Copy link
Collaborator

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 JohnAlbin added area: next-drupal and removed triage A new issue that needs triage labels Feb 22, 2024
@lorenzo-dallamuta
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: next-drupal bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants