Using generic in baseclass about polytype HOT 17 CLOSED

So, angular 14 was released sometime last week - and I tried out the approach you suggested above

But it seems to give me an error: Value of type 'typeof BaseClass' is not callable. Did you mean to include 'new'?(2348)

I created a reproducible stackblitz here: https://stackblitz.com/edit/angular-14-polytype?file=src%2Fapp%2Fapp.component.ts
Could you check if I am using polytype correctly ?

from polytype.

In TypeScript 4.7,

export class UserService extends classes(WriteService<UserModel>) {
    // ...
}

is now allowed. The syntax WriteService<UserModel> for a value is what they call an instantiation expression, something that TypeScript has been missing for a really long time.

If you work with an older version version of TypeScript, the simplest workaround I'm aware of is introducing an intermediate class:

class UserWriteService extends WriteService<UserModel> {
}

export class UserService extends classes(UserWriteService) {
}

As a note, support for TypeScript < 4.7 will be removed in a future version of Polytype, so if you have a chance to upgrade TypeScript to the latest version now, that would be the best.

from polytype.

For typescript 4.7 - looks like I will have to wait for angular 14 to be released based on the compatibility matrix at:
https://gist.github.com/LayZeeDK/c822cc812f75bb07b7c55d07ba2719b3
Looks like it might be right around the corner as they are already in an RC2 - https://github.com/angular/angular/releases

The UserWriteService approach seems to be cumbersome - but I get it.

I did find another solution which seems to work:

export interface UserService extends WriteService<UserModel>;

export class UserService extends classes<any>(WriteService) {
}

The classes<any> makes the actual class ignore the typing from there (If not done it conflicnts with the interface).
And I can provide typing separate with declaration merging

from polytype.

Your app.component.ts looks fine to me. I tried running

npx tsc --target es2017 --moduleResolution node --experimentalDecorators src/app/app.component.ts

and it worked perfectly with TypeScript 4.7.3. Probably StackBlitz is still using an older version of TypeScript for code highlighting, but with the latest version there are no errors.

from polytype.

Thanks - you were right. Looks like even my VSCode is using an older version of typescript (2.6) for the TS errors.
I assume stackblitz linter was using the older typescript too :)

Thanks!

from polytype.

No problem and thanks for your feedback.

from polytype.

@fasttime I had earlier used the workaround because angular / vscode were using typescript <4.7
But now, vscode uses 4.9 and angular14 uses 4.8
So, I was trying to remove my workarounds and noticed this was still not working how I expected it to work.

There is no syntax error anymore. But the function .getAll() seems to be returning any[] instead of the {id: number}[] I was expecting it to show:

This is reproducible in the same stackblitz as earlier:
https://stackblitz.com/edit/angular-14-polytype
(Looks like stackblitz has also upgraded to typescript 4.8+ as it works there now)

from polytype.

Hm, it seems that you need to specify "moduleResolution": "Node16" or "moduleResolution": "NodeNext" in the "compilerOptions" of the tsconfig file (this works also in Node.js 14 despite the name). The module resolution used in Polytype has changed in version 0.15 and I totally missed to document the setting. "NodeNext" is supposed to become the new default one day but for now it needs to be specified manually.

But there is another problem: StackBlitz doesn't know how to resolve the .d.ts file despite the correct tsconfig settings, probably because it uses the Monaco editor of Visual Studio Code, which has the same issue. That is why it's treating all imports from Polytype as if they had type any. I don't know if/how this can be changed at this time.

Alternatively, you could revert to the previous version 0.14.1 which is not affected by this problem, as long as the editor has not implemented the new module resolution.

I'm sorry for the inconvenience. Keep in mind that this library has not reached a stable release yet, and that's simply because the technology is still changing.

from polytype.

Hey there, thanks for the quick replies !

Firstly, I feel like polytype is the most robust implementation of its kind to solve my needs. So, minor inconveniences are not a issue. Happy to debug with you to make it more stable !

Second, I am still using 0.14.1 (My stackblitz happened to be on 0.15)
I have used 0.14.1 on my local and also in stackblitz and the issue is still there:

In my 0.14.1 setup locally, I tried to configure: "moduleResolution": "Node16", and that still did not fix the issue for me
I tried the same in stackblitz (i.e. moduleResolution: Node16 + polytype 0.14.1) and the issue still exists

Do you happen to have a working stackblitz example I could inspect and see what could be the culprit

from polytype.

Ah okay, thanks for the clarifications. In this case I'll have to take a closer to see what is wrong.

polytype is the most robust implementation of its kind

I know... 😉

from polytype.

I fixed a problematic type definition and released new version 0.16.0. @AbdealiLoKo could you please test this again? It's showing this for me when I hover over cls.getAll();

(method) BaseClass<{ id: number; }>.getAll(): {
    id: number;
}[]

Unfortunately, I haven't managed to get this working in the online editor. As already mentioned, it seems that StackBlitz doesn't like "moduleResolution": "Node16".

from polytype.

I upgraded to polytype 0.16 in my project.

When I try to use "moduleResolution": "Node16" it starts to give me errors in my existing imports
The first import in every file (mostly angular imports like: import { Injectable } from '@angular/core';)
VSCode started giving:

The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'. Consider writing a dynamic 'import("@angular/core")' call instead.
  To convert this file to an ECMAScript module, change its file extension to '.mts', or add the field `"type": "module"` to '/repos/myrepo/package.json'.ts(1479)

I did not want to mess with the import things because I am not very proficient with them.
So, I reverted back to our old value of "moduleResolution": "node"
And then I get this error in VSCode:

Could not find a declaration file for module 'polytype'. '/repos/myrepo/node_modules/polytype/lib/polytype.cjs' implicitly has an 'any' type.
  Try `npm i --save-dev @types/polytype` if it exists or add a new declaration (.d.ts) file containing `declare module 'polytype';`

VSCode: 1.74.2
Typescript: 4.8.4
Angular: 14.2.5

EDIT: I will speak to a friend who helped me setup the tsconfig to understand this a bit better but if you have any leads, that would be awesome

EDIT: I tried adding "moduleResolution": "Node16", "type": "module" to my tsconfig but that did not help

from polytype.

Thanks a lot for testing @AbdealiLoKo! It seems the error you are receiving with "moduleResolution": "Node16" is related to other settings of the package configuration that start to become important when the module resolution changes.

I was able to fix the error by setting "type": "module" in package.json. I also had to change the versions of all @angular packages in the "devDependencies" to make them match the version in the "dependencies" (Yours is "^15.0.4"). Finally, I added a "devDependency" on "ajv", required to support npm <7. The result is here: https://stackblitz.com/edit/angular-14-polytype-evzdvv?file=package.json
With these changes, I was able to run npm install and then npm run serve and get the Angular test app running on my local machine.

Now, for an existing real-world application, the changes required to switch module resolution could be very complex, if possible at all (I didn't even take care of karma.conf.js). Plus, editor support is still very poor: StackBlitz doesn't recognize "moduleResolution": "Node16" and VSCode complains about relative imports in app.module.ts. For these reasons I'm thinking that changing the module resolution in Polytype so early was maybe not the best decision.

from polytype.

I tried downloading your stackblitz and opened it up locally - and that works correctly 👍

Now what would be your recommendation ?

• Do you plan on supporting the older moduleResolution: node with this fix ? Or maybe backporting the fix to v1.14.x series ?
• Or do I need to figure out how to move my application to use type: module ?

I tried the type: module and as you said karma.conf.js, .eslintrc.js, and even some of my custom eslint rules etc started throwing errors as everything is being processed as a ES module instead of CommonJS module ... so it would be a fair amount of work to make everything work

from polytype.

Just re-added support for "moduleResolution": "node" in version 0.16.1. I simply reverted some of the changes in 6363f5c. This should hopefully work well for the time being. I'm still unsure whether to keep the rest of the work done to support "Node16" or undo it completely until support for the new module resolution improves in the ecosystem.

from polytype.

Woot ^_^/
It works ! 500 lines of explicit typing deleted 🥳

from polytype.

Great! Thanks for reporting and testing this, your help is very appreciated 👍🏻

from polytype.