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

Error in generated typescript definitions #91

Closed
Toeler opened this issue May 31, 2024 · 9 comments
Closed

Error in generated typescript definitions #91

Toeler opened this issue May 31, 2024 · 9 comments

Comments

@Toeler
Copy link

Toeler commented May 31, 2024

In the @ngrx-traits/signals package, the with-state-logger and with-sync-to-web-storage files are generating type definitions that fail to compile.

This can be seen here: https://stackblitz.com/edit/ngrx-signal-store-starter-7bcgrp?file=node_modules%2F%40ngrx-traits%2Fsignals%2Flib%2Fwith-sync-to-web-storage%2Fwith-sync-to-web-storage.d.ts

I'm not sure why this still is able to run, but if you right click on withSyncToWebStorage in the imports and view the definitions, you will se the error. In my personal project I can't even get it to run if I import any functions at all from @ngrx-traits/signals, even if I don't use them.

The generated definition file is something like this:

export declare function withSyncToWebStorage<Input extends SignalStoreFeatureResult>({ key, type: storageType, saveStateChangesAfterMs, restoreOnInit, filterState, onRestore, }: {
    key: string;
    type: 'session' | 'local';
    restoreOnInit?: boolean;
    saveStateChangesAfterMs?: number;
    filterState?: (state: Input['state']) => Partial<Input['state']>;
    onRestore?: (store: Prettify<SignalStoreSlices<Input['state']> & Input['signals'] & Input['methods'] & StateSignal<Prettify<Input['state']>>>) => void;
}): import("@ngrx/signals").SignalStoreFeature<import("@ngrx/signals/src/signal-store-models").EmptyFeatureResult & Input extends infer T ? { [K in keyof T]: (import("@ngrx/signals/src/signal-store-models").EmptyFeatureResult & Input)[K]; } : never, {
    state: Omit<Omit<{}, "saveToStorage" | "loadFromStorage" | "clearFromStore">, never>;
    signals: Omit<Omit<{}, "saveToStorage" | "loadFromStorage" | "clearFromStore">, never>;
    methods: Omit<Omit<{}, "saveToStorage" | "loadFromStorage" | "clearFromStore"> & {
        saveToStorage(): void;
        loadFromStorage(): boolean;
        clearFromStore(): void;
    }, never>;
} & import("@ngrx/signals/src/signal-store-models").EmptyFeatureResult>;

With the error

Type 'EmptyFeatureResult & Input extends infer T ? { [K in keyof T]: (EmptyFeatureResult & Input)[K]; } : never' does not satisfy the constraint 'SignalStoreFeatureResult'.
  Type '{}' is missing the following properties from type 'SignalStoreFeatureResult': state, signals, methods(2344)
@gabrielguerrero
Copy link
Owner

@Toeler , I'm failing to duplicate this, even in your stackblitz I dont see any error, are you still having the issue ?

@gabrielguerrero
Copy link
Owner

Maybe it could be your IDE, have you tried restarting the TS service or even the IDE

@Toeler
Copy link
Author

Toeler commented May 31, 2024

In the Stackblitz it isn't failing to compile, but the error is visible when viewing the typescript definition.
Screenshot_20240531_230818_Brave

I haven't yet figured what in my apps build process is causing it to fail it but it's definitely the same error.
I was also able to reproduce the error by building the package from source and checking the definition in my IDE.

My temporary workaround was to add skipLibChecks to my tsconfig but that isn't a permanent fix.

@gabrielguerrero
Copy link
Owner

I just notice something in the stactkblitz not sure is the same in your env , the typescript version is old 4.0.2, this depends on angular 17 so typescript must be 5.x could this be the problem in your env?

@Toeler
Copy link
Author

Toeler commented May 31, 2024

Ahh, good eye. I just used a pre-built Stackblitz from the @ngrx/signals github and modified it.
But that is a red herring. my project is running ~5.3.3 as is the fresh install of the @ngrx-traits source code that I have.

I think it is to do with how typescript tries to import the implied return type of the two functions. All of the other features that this package exports have explicit return types.

Here is a quick patch which uses some types that I pulled from the generated definition file. This fixes the errors.
ngrx-traits-10-46-30-AM.patch

@gabrielguerrero
Copy link
Owner

Hey thanks for the patch, yeah that looks like should work, I will test it , but Im still intrigued I can not duplicate in my projects does not happen, Im investigating maybe is tsconfig difference

@gabrielguerrero
Copy link
Owner

gabrielguerrero commented Jun 1, 2024

ok my projects have skipLibCheck : true , I tried generating new angular projects using ng new and its like that by default, which is probably why my projects have it, I think is by default set to true because it speeds up compilation, otherwise is type checking all node_modules, but anyway the library should still work with it on false so Ill test the fix

@gabrielguerrero
Copy link
Owner

Forgot to update on this, I tested a fix for this weeks ago, but it causes a bug in intellij j in their new TS engine that makes the intellisense in the angular templates not work, is an intellij bug because the intellij sense outside the template works fine, and in vscode there is no problems at all. What I been doing is every time the do a new intellj release Im checking if it fixes the problem. So for now I think the intellj bug will affect more users because skipLibCheck: is true by default , so Im holding the fix for now

gabrielguerrero pushed a commit that referenced this issue Oct 1, 2024
Made the return type explicit for withSyncToWebStorage, withStateLogger and
withSyncToRouteQueryParams, before the were inferred which worked fine in the examples inside the
project but when compiled as lib, causes a problem were the functions inside the traits can not
correctly infer the store , this also fixes using the library with skipLibCheck: false

Fixes #145 , #91
gabrielguerrero pushed a commit that referenced this issue Oct 1, 2024
Made the return type explicit for withSyncToWebStorage, withStateLogger and
withSyncToRouteQueryParams, before the were inferred which worked fine in the examples inside the
project but when compiled as lib, causes a problem were the functions inside the traits can not
correctly infer the store , this also fixes using the library with skipLibCheck: false
Added missing docs, for typedCallConfig

Fixes #145 , #91
gabrielguerrero pushed a commit that referenced this issue Oct 1, 2024
Made the return type explicit for withSyncToWebStorage, withStateLogger and
withSyncToRouteQueryParams, before the were inferred which worked fine in the examples inside the
project but when compiled as lib, causes a problem were the functions inside the traits can not
correctly infer the store , this also fixes using the library with skipLibCheck: false
Added missing docs, for typedCallConfig

Fixes #145 , #91
@gabrielguerrero
Copy link
Owner

Forgot to close this, it was fix by #145 and #146

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

No branches or pull requests

2 participants