greenpress / vuex-composition-helpers

A util package to use Vuex with Composition API easily.
https://www.npmjs.com/package/vuex-composition-helpers
MIT License
288 stars 32 forks source link

TS error: Implicit conversion of a 'symbol' to a 'string' will fail at runtime #80

Open baffalop opened 3 months ago

baffalop commented 3 months ago

I am upgrading a codebase to Vue 3, so have upgraded vuex-composition-helpers to 2.0.3.

When running unit tests for components using this library, I get a typescript error:

 ● Test suite failed to run

    node_modules/vuex-composition-helpers/src/util.ts:49:48 - error TS2731: Implicit conversion of a 'symbol' to a 'string' will fail at runtime. Consider wrapping this expression in 'String(...)'.

    49   return cb(store, namespace ? `${namespace}/${prop}` : prop);

Because the library is uncompiled typescript, I have to include it in my Jest transform, with:

{
  transformIgnorePatterns: [
    'node_modules/(?!vuex-composition-helpers)'
  ],
}

This means that the library is typechecked as part of my unit tests.

Versions

vuex-composition-helpers: 2.0.3 typescript: 5.2.2 jest: 29.7.0 ts-jest: 29.1.1 @vue/vue3-jest: 29.2.6

davidmeirlevy commented 3 months ago

Hi!

How would you suggest to fix this bug? Can you create a demo stackblitz with this bug that we can play with and figure out a solution?

Thanks.

baffalop commented 3 months ago

Thanks for looking! I’m on holiday for a week but can provide a reproduction when I’m back.

I haven’t taken a proper look at the code, but I imagine maybe the functions involved unintentionally accept symbol and might want to exclude it from the type for prop? I don’t know what happens when a symbol is explicitly cast to a string but maybe that would work and the suggestion from the error message to wrap with String(…) is sufficient?

davidmeirlevy commented 3 months ago

@baffalop any demo app with as minimal code as possible that can make this error to appear is sufficient.

A better way is to create a unit test to mock a minimal app with a demo state to retrieve the expected result you want it to have. We'll make this unit test pass.

baffalop commented 2 months ago

@davidmeirlevy Here is a reproduction with a single unit test which fails because of the type error:

https://stackblitz.com/edit/vue3-vuex-vite-typescript-starter-qzr8gm?file=test%2FHelloWorld.spec.ts&view=editor

(Run npm test to see the error)

baffalop commented 1 month ago

I came back to this to see if I could contribute a PR, but now I realise that it's a much more mysterious issue than I thought. I expected to be able to reproduce it just by bumping the typescript version, but nope—and quite reasonably, because the KnownKeys type explicitly excludes symbol!

export declare type KnownKeys<T> = Exclude<KnownKeysWithAllPrimitiveTypes<T>, symbol>

And the prop paramter of runCB has the type KnownKeys<T> | string.

My only guess at this point is that ts-jest is responsible for the error somehow? @davidmeirlevy Did you get any further when investigating this?

baffalop commented 1 month ago

Update: I just realised that the KnownKeys type I'm seeing when I pull the next branch is not the source I see when I navigate to node_modules/vuex-composition-helpers in my own codebase. Namely, what I see in my node_modules is:

export declare type KnownKeys<T> = {
    [K in keyof T]: string extends K
        ? (T extends any ? any : never)
        : number extends K
            ? never
            : K
} extends {
        [_ in keyof T]: infer U
    }
    ? U
    : never;

This is the old type which as part of https://github.com/greenpress/vuex-composition-helpers/commit/56a2acc840355ac5944acd7b7b645787a8d9e15c was refactored to KnownKeysWithAllPrimitiveTypes.

I have v2.0.3 installed, which matches what gets installed when requesting the @next tag. However, this version doesn't seem to include that change. I'm not actually sure which version of the code gets installed... I see a git tag for 2.0.1, and I also see that the HEAD of the next branch has 2.0.2 declared in its package.json. (I tried installing 2.0.2 but the change doesn't seem to be there either.)

I'm so confused. It seems the fix is out there but just does not get installed by npm. I don't have experience with library authorship so I don't know how npm resolves versions. Can you shed any light on this?

baffalop commented 1 month ago

I was able to bypass the issue by installing the fork @wisdomgarden/vuex-composition-helpers which has a version 2.0.2-1 with the latest code.

I think this issue might boil down to: the code published under versions 2.0.2 and 2.0.3 does not reflect the latest src.

radek-furmanski commented 1 month ago

I have the same issue. Installing fork version as @baffalop suggested seems to resolve the problem. Could you guys ensure that the newest version of the lib has the necessary changes?

baffalop commented 1 month ago

@radek-furmanski worth noting that another way to bypass the typescript errors in Jest is to use the dist js version of vuex-composition-helpers, ie. adding the following to jest.config.js:

moduleNameMapper: {
  'vuex-composition-helpers': '<rootDir>/node_modules/vuex-composition-helpers/dist/index.js'
}

This is what I went with in the end rather than using the fork.