bloomreach / spa-sdk

Apache License 2.0
16 stars 15 forks source link

Export Menu10 to avoid TS4058 #40

Closed sethidden closed 1 month ago

sethidden commented 1 month ago

Here you're defining Menu: https://github.com/bloomreach/spa-sdk/blob/2b4336e0b45f0fbf92cc8136147d5253ea8fce9a/packages/spa-sdk/src/page/menu.ts#L51 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 commented 1 month ago

Please see my PR: https://github.com/bloomreach/spa-sdk/pull/41

cc @hachokbloomreach

hachok commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

hachok commented 1 month ago

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

sethidden commented 1 month ago

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