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

Export Menu10 to avoid TS4058 #40

Closed
sethidden opened this issue Jul 12, 2024 · 8 comments
Closed

Export Menu10 to avoid TS4058 #40

sethidden opened this issue Jul 12, 2024 · 8 comments

Comments

@sethidden
Copy link

sethidden commented Jul 12, 2024

Here you're defining Menu:

export interface Menu {

And here you're using it:
https://github.com/bloomreach/spa-sdk/blob/2b4336e0b45f0fbf92cc8136147d5253ea8fce9a/packages/spa-sdk/src/page/
menu.ts#L113

The problem is that if you have code like this in TypeScript 5.5:

import { isMenu as bloomreachIsMenu } from "@bloomreach/spa-sdk";
export function isMenu(content: Content) {
  return bloomreachIsMenu(content);
}

You'r run into error TS4058 saying:

error TS4058: Return type of exported function has or is using name 'Menu$2' from external module "/Users/rt/dev/enterprise/node_modules/@bloomreach/spa-sdk/dist/index" but cannot be named.

The problem here is that somewhere in the code there's another interface named Menu, because the interface in isMenu is Menu$2:

obraz obraz

The resolution to this TS4058 error would be to import { Menu$2 } from '@bloomreach/spa-sdk' however I can't do that because it's not exported.

Please rename the interfaces so their names don't clash (There's Menu, Menu$1 and Menu$2) so they clash two times and export them

@sethidden sethidden changed the title Export Menu$2 to avoid TS4058 Export Menu10 to avoid TS4058 Jul 12, 2024
@sethidden
Copy link
Author

Please see my PR: #41

cc @hachokbloomreach

@hachok
Copy link
Collaborator

hachok commented Jul 15, 2024

Hey @sethidden!
I couldn't reproduce this issue. I tried it with vue@3.4.31, @vue/tsconfig@0.5.1, typescript@5.5.3, vue-tsc@2.0.26
What stack are you using?

I noticed that you did not import type Content from "@bloomreach/spa-sdk". Is there a reason for this?
By the way, you can use getContent<Menu> to ensure the types match the type Menu.

@sethidden
Copy link
Author

sethidden commented Jul 15, 2024

@hachok Hello, I've made a repo with a repro case.

One liner for repro should be:
git clone https://github.com/sethidden/bloomreach-spa-sdk-is40-repro.git && cd bloomreach-spa-sdk-is40-repro && npm ci && npm run build

Or you can just see the code at https://github.com/sethidden/bloomreach-spa-sdk-is40-repro.git


added 12 packages, and audited 13 packages in 624ms

7 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

> bloomreach-repro@1.0.0 build
> tsc

src/index.ts:3:17 - error TS4058: Return type of exported function has or is using name 'Menu$2' from external module "/Users/rt/dev/bloomreach-spa-sdk-is40-repro/node_modules/@bloomreach/spa-sdk/dist/index" but cannot be named.

3 export function isMenu(content: Content) {
                  ~~~~~~


Found 1 error in src/index.ts:3

@hachok
Copy link
Collaborator

hachok commented Jul 15, 2024

I see that the issue does not occur when compilerOptions.noEmit is set to true. Would this solution work for you?
We'll further investigate how to handle the issue.

@sethidden
Copy link
Author

Unfortunately it's not possible in our case. Our package is a library that uses @bloomreach/spa-sdk underneath, and we can't set noEmit to true because otherwise the d.t.s files would not be generated for the library.

@hachok
Copy link
Collaborator

hachok commented Jul 15, 2024

I see! I'll add your #41 change in the next release @bloomreach/spa-sdk@23.3.1 and will notify you once it's released.

ajbanck pushed a commit that referenced this issue Jul 24, 2024
@hachok
Copy link
Collaborator

hachok commented Jul 24, 2024

@bloomreach/spa-sdk@23.3.1 has been released, containing the fix!

@hachok hachok closed this as completed Jul 24, 2024
@sethidden
Copy link
Author

Just updated and everything is working correctly. Thanks for taking care of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants