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

Support "external files" provided from TypeScript LS plugins #25740

Closed
RyanCavanaugh opened this issue May 1, 2017 · 8 comments
Closed

Support "external files" provided from TypeScript LS plugins #25740

RyanCavanaugh opened this issue May 1, 2017 · 8 comments
Assignees
Labels
feature-request Request for new features or functionality typescript Typescript support issues
Milestone

Comments

@RyanCavanaugh
Copy link
Member

Tracking work for supporting non-TS files for TypeScript language service plugins

PRs this blocks:
microsoft/TypeScript#15308
https://github.com/chuckjaz/angular/tree/ts_plugin_update

/cc @chuckjaz @mjbvz

@kieferrm kieferrm added typescript Typescript support issues feature-request Request for new features or functionality labels May 1, 2017
@mjbvz mjbvz added this to the May 2017 milestone May 5, 2017
@kieferrm kieferrm mentioned this issue May 5, 2017
44 tasks
@mjbvz
Copy link
Contributor

mjbvz commented May 8, 2017

My current thinking is that we add a handles section typescriptServerPlugins contribution, something like:

"contributes": {
    "typescriptServerPlugins": [
        {
            "name": "tslint-language-service",
            "handles": {
                "extensions": [
                    ".html",
                    ".ng-html"
                ],
                "modeIds": [
                    "html"
                ]
            }
        }
    ]
}

extensions is a list of file extensions while modeIds is a list of vscode language modes.

Another version of this contributions specification where the extension defines all handled languages at the top level:

"contributes": {
    "typescriptServerPlugins": {
        "plugins": [
            {
                "name": "tslint-language-service"
            }
        ],
        "handles": {
            "extensions": [
                ".html",
                ".ng-html"
            ],
            "modeIds": [
                "html"
            ]
        }
    }
}

@chuckjaz + @egamma Would this meet your needs? Any recommended changes or additions?

@chuckjaz
Copy link

chuckjaz commented May 8, 2017

Maybe language id's instead of extensions but otherwise looks good to me.

@egamma
Copy link
Member

egamma commented May 9, 2017

  • Why do you need both extensions and modeIds? This should be done in terms of language Ids and not file extensions otherwise it will not work when the user customizes file associations.
  • modeIds should be languageIds to be consistent with the rest of the API

mjbvz added a commit to mjbvz/vscode that referenced this issue May 10, 2017
Part of microsoft#25740

To support TS Server plugins for languages like angular, we will allow extensions to register new langauges for TypeScript to watch. The angular language for example would want ng-html files to also be uploaded to TypeScript for checking

The current language definitions all define both a set of language modes they support and a set of file extensions. The file extension part is unnessiary and may be incorrect depending on how a user sets up their `file.associations` in the workspace. This change removes the extensions part so that we always make use of the language mode
mjbvz added a commit that referenced this issue May 11, 2017
…26413)

Part of #25740

To support TS Server plugins for languages like angular, we will allow extensions to register new langauges for TypeScript to watch. The angular language for example would want ng-html files to also be uploaded to TypeScript for checking

The current language definitions all define both a set of language modes they support and a set of file extensions. The file extension part is unnessiary and may be incorrect depending on how a user sets up their `file.associations` in the workspace. This change removes the extensions part so that we always make use of the language mode
@mjbvz
Copy link
Contributor

mjbvz commented May 11, 2017

Ok, here's an updated schema proposal:

"contributes": {
    "typescriptServerPlugins": [
        {
            "name": "tslint-language-service",
            "languages": [ "html" ]
        }
    ]
}

@mjbvz mjbvz closed this as completed in 4f398ba May 16, 2017
@mjbvz
Copy link
Contributor

mjbvz commented May 16, 2017

I've pushed a prototype implementation of this with 4f398ba. It uses the contribute specification described above. Here's standard TS error reporting in an html file for example:

screen shot 2017-05-15 at 4 57 05 pm

This should be in the next insiders build

I'll keep iterating on this on our side to improve the UX and to better handle some edge cases. Please try it out and let me know if you run into any issues

@chuckjaz
Copy link

I validated this works.

I built a version of vscode from sources following the instructions in https://github.com/Microsoft/vscode/wiki/How-to-Contribute#build-and-run-from-source synced to synced to c15e2b0.

I build a version of TypeScript that includes microsoft/TypeScript#15308 rebased to 7473dcc0410f9865295daeb9bc534f7efdbcb290.

I created a vscode extension that declares @angular/language-service as a TypeScript plugin as described above.

I verified that request from html files are directed to tsserver and that the @angular/language-service is processing the request.

@mjbvz
Copy link
Contributor

mjbvz commented May 17, 2017

Thanks @chuckjaz

One big problem with the current VSCode implementation is that if an extension registers the html language, this causes TypeScript to start reporting errors and providing intellisense in every html file, not just html files that angular for example may care about. @RyanCavanaugh / @mhegazy Any thoughts on how we should handle this? Right now we just send an open request that looks like:

[Trace  - 5:51:39 PM] Sending request: open (3). Response expected: no. Current queue length: 4
Arguments: {
    "file": "/Users/matb/projects/test-vscode-tsserver-plugin/x.html",
    "fileContent": "#test",
    "projectRootPath": "/Users/matb/projects/test-vscode-tsserver-plugin"
}

Could we add a new scriptKind or argument to open to identify external files?

@RyanCavanaugh
Copy link
Member Author

@mjbvz let's discuss tomorrow

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality typescript Typescript support issues
Projects
None yet
Development

No branches or pull requests

5 participants