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

[Feature] Add Language Service support #85

Closed
WalterWoshid opened this issue Nov 25, 2022 · 10 comments · Fixed by #96
Closed

[Feature] Add Language Service support #85

WalterWoshid opened this issue Nov 25, 2022 · 10 comments · Fixed by #96

Comments

@WalterWoshid
Copy link

@nonara I hope you don't mind if I create a new issue for this.


So, I've been debugging like crazy and I found out that the transformer is actually working (VSCode) just by modifying this line in src/patch/lib/createProgram.ts:89:

if (tsp.isTSC || tsp.isTSServer) {

I have implemented tsp.isTSServer just the way tsp.isTSC has been implemented (checking if the file name matches).


For debugging I used VSCode and downloaded the Restart TS server Status Bar button and the VS Code RPC Server extension, set node for the rpcServer.nodeDebugger.debugAdapter setting in VSCode and added Easy-Attach breakpoints in my plugin and in the tsserver file, which worked great :) (you should include it in the docs, if you want, it's really the easiest way to debug this plugin, the typescript core code and the transformer itself)

I think I can achieve my previous goal of modifying the module name resolution of TypeScript, just need to dig a little deeper


TypeScript version: 4.7.4

@WalterWoshid WalterWoshid changed the title Working with tsserver.js Working with TypeScript Language Server (IDE's) Nov 25, 2022
@WalterWoshid
Copy link
Author

Oh and for JetBrains WebStorm the transformer is also working. Here its not the tsserver.js file, but the tsserverlibrary.js and I also had to add tsp.isTSServerLibrary to make it work

@WalterWoshid
Copy link
Author

WalterWoshid commented Nov 25, 2022

And weirdly enough I have not encountered any bugs or errors. The only confusing thing that looked like a bug is the TypeScript Output from VSCode. This line [Error - 22:09:27.342] TSServer exited. Code: null. Signal: SIGTERM which appears after restarting the TS Server, but I figured that the message is from the previous instance, just delayed. That gave me a good headache, before I figured it out

@nonara
Copy link
Owner

nonara commented Nov 26, 2022

Nicely done!

Regarding the PR, I think I'd prefer we make a new field moduleName, which would be the filename minus the extension (ie. tsserver)

To avoid a breaking change, we can leave isTSC, but not use it in the createProgram patch.

If you want to enable me making commits to the PR, I'll jump in and make a few tweaks!

As for your goal, it sounds like the perfect use case for the rewrite I have planned out. I have a lot going on so I don't have the time to do it all on my own.

Seems like you know what you're doing! Would you be interested in helping write the new version? It would allow you a means of patching TS in the way you want that would be maintained as future TS versions are released. If it aligns with your goals too and makes sense, I'd be glad to work together to make it happen.

Short version of the rewrite is, it will allow applying custom patches to TS itself, which will help your application. Also, all plugins will be single packages which can contain multiple transformers, language server plugins, and patches.

Thinking of also providing another custom plugin type which provides an easy way to filter / ammend Diagnostics (error messages), as that's highly requested, but many don't know how to do it on the LS side.

Either way, I'd be interested in hearing your ideas for patching TS. I haven't decided yet what the best route would be for that. I was planning on probably doing transformers, but if I am remembering correctly, I think you mentioned another way of patching.

I'm on mobile now, but I can talk more about it and push through the PR in the next few days.

@WalterWoshid
Copy link
Author

Thank you.

Of course you can tweak it!

I'd love to help, but I am not sure yet how these custom patches should look like or how you imagine it exactly. Do you have a platform for chatting like teams, discord, slack, etc.? Anything is good enough for me.

@WalterWoshid
Copy link
Author

One of my ideas would be to expose internal functions and/or variables. For example the ts.createTypeChecker().resolveName() function does something that I need to check/override. Trying to create a proxy (override the original function) for createTypeChecker works perfectly, but it returns a variable called checker with the resolveName() function, which is only a wrapper method for the ts.createTypeChecker().resolveName() and it never gets called. The internal resolveName() is called by getResolvedSymbol() which is called by checkIdentifier() which is called by ... 23 more internally called functions ... which is called by getSemanticDiagnostics(), which I CAN override, but from there I can't check what I wanted to check.

My solution for this problem, which is probably your "custom plugins" idea, is to have a custom plugin that allows you to add extension points to internal variables and functions. It could be done by writing code like this: ExtensionPoint.add("ts.createTypeChecker().resolveName()") and the patcher would apply the extension point at runtime or per CLI. You could then use ExtensionPoint.listen("ts.createTypeChecker().resolveName()", customOverrideCallback) to catch the internal function call. Just an idea, but this would allow for a lot of customizability.

@nonara nonara changed the title Working with TypeScript Language Server (IDE's) [Feature] Add Language Service support Jan 6, 2023
@Griffork
Copy link

Griffork commented Jan 8, 2023

Is this usable? I'd like to use tsserver/tsserverlibrary with intellij but it's hard to find information for how well supported it is.

@nonara
Copy link
Owner

nonara commented Jan 11, 2023

@Griffork there is a PR that works. Just waiting on tests to merge:

@davidskuza
Copy link

Hello @nonara Are you sure this PR works right now? Or how should I run this? I tried to patch and run locally to test some things but only tsc.js and typescript.js gets patched anyway (https://github.com/nonara/ts-patch/blob/master/src/installer/lib/actions.ts#L25) and just adding tsserver.js doesn't help because it errors at "Could not find tsc executeCommandLine". Not sure also if the same patches should even apply to all those files the same way yet.

@nonara
Copy link
Owner

nonara commented Feb 8, 2023

@davidskuza You can use ts-patch patch tsserver.js to directly patch it. According to the author, it works. I tested some, but not extensively.

Not sure also if the same patches should even apply to all those files the same way yet.

It should.

Were you trying with TS 5? tsp doesn't support 5 yet.

@nonara nonara mentioned this issue Jun 13, 2023
Merged
@nonara nonara closed this as completed in cd69c1c Jun 13, 2023
@nonara
Copy link
Owner

nonara commented Jun 13, 2023

This is now added in v3. It works on our end, but I can't guarantee it will have the desired effect in IDEs, and IDEs often do some weird patching of TS themselves.

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

Successfully merging a pull request may close this issue.

4 participants