-
Notifications
You must be signed in to change notification settings - Fork 90
Export getTypeScriptWorker & getJavaScriptWorker to monaco.languages.typescript #8
Conversation
Hi @paveldk, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
35de1f3
to
1885b53
Compare
ping @jrieken |
const worker = (first: Uri, ...more: Uri[]): Promise<TypeScriptWorker> => { | ||
return client.getLanguageServiceWorker(...[first].concat(more)); | ||
}; | ||
client = new WorkerManager(defaults); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work cos the method is being called twice which means either JS or TS wins and you never know what you got. I'd make the setupMode
method return the client and inside setupTypeScript
resp setupJavaScript
set the proper state for getJavaScriptWorker
and getTypeScriptWorker
. Before that those method should simply reject
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, just figured out the same thing a couple of hours ago. Will apply fix now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be just one worker for both JS and TS or 2 separate? If they are separate wouldn't the completion be laggy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two workers as we have it now. Since they are scoped one is asked for JS files and the other for TS files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see how this is better and I can agree with this approach. But if we want to have cross-js-ts completions we either have to use one worker here or change how monaco works with SuggestAdapters and concat results from multiple that can be responsible for certain fail type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, both are two separate 'extensions' and cannot be married in that way cos it will disallow having different configurations (compiler options, extra libs) for each. That doesn't mean you cannot have JS files in TypeScript - use the allowJS
-options and add them as extra lib
or just add JS content as TS-file.
5554eb2
to
26c6e58
Compare
3fc9491
to
e55e12e
Compare
@jrieken applied changes -> now exports the workers and all functions are available for use |
@@ -53,6 +76,8 @@ function setupMode(defaults:LanguageServiceDefaultsImpl, modeId:string, language | |||
disposables.push(new languageFeatures.DiagnostcsAdapter(defaults, modeId, worker)); | |||
disposables.push(monaco.languages.setLanguageConfiguration(modeId, richEditConfiguration)); | |||
disposables.push(monaco.languages.setTokensProvider(modeId, createTokenizationSupport(language))); | |||
|
|||
return client; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of the client
you could export the worker
function here. It already does the right thing. Also note that you should have a signature that takes n
resources otherwise there framework might drop them again
e55e12e
to
1bf301f
Compare
LGTM |
Works nicely, however I'm adding some additional libraries using monaco.languages.typescript.typescriptDefaults.addExtraLib function and when I compile using your proposed solution where the model is derived from the editor (editor.getModel()) then it only compiles what's written in the editor and everything that is added using addExtraLib function is ignored in the compiled output. Is it possible to have compiler including also the files added using addExtraLib function so the compiled output contains everything in the context and not only whats written in the editor? I guess it would be possible to manually concatenate everything that was added as extra libs including the editor content to a single model and then have it compiled but it seems rather manual. |
@ThorConzales |
I solved it by concatenating everything to a single file and compiling it with typescript-simple. |
Is there any docs on this yet? What functions are exposed? I was hoping to get access to the language service itself but it seems you made proxies using promises instead. Some TypeScript definitions would have been enough; not sure why the service returns "any" when it could have returned the promise structures. |
After some more research and reviewing the debugger, I created this if it helps anyone:
|
The idea behind this pull request is to enable and export the typescript compilation for multiple files.
You can now create multiple models with:
(will be good idea to cache those)
In this way, you will feed the typescriptService host with all your available files and then you can simply call:
Which will compile the file you have already created Model for.
We are attaching the function to the Monaco.languages.typescript object so we can use it for multiple files/models and not just the current one.
As this is my first PR I want to make a small step before exposing the whole API (will probably iterate through the worker functions and add those to exports)
Related to microsoft/monaco-editor#35