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

Add vue.d.ts and typescript support #94

Merged
merged 13 commits into from Mar 21, 2017

Conversation

Projects
None yet
4 participants
@sandersn
Contributor

sandersn commented Mar 16, 2017

  1. Add an implicit import Vue from 'vue' and new Vue(...) around the default exported object literal. This provides a lot better intellisense than just a bare object literal because the Typescript language server knows that the object will be used by Vue.
  2. Resolve other .vue files so that import other from './other.vue' provides correct info.
  3. If lang='typescript' is specified, put the TS language service in TS mode: all errors are reported and completions include only symbols that are known to be correct.
  4. Also fix a bug in htmlServerMain so that errors are actually reported. Looks like this is already fixed in master.

Fixes #75

sandersn added some commits Mar 16, 2017

Add dependency on vue-template-compiler
Probably it should replace the space-replacing
code for individual regions with a one-shot parsing that gets looked up
for each mode as updateCurrentTextDocument is called.
Add vue.d.ts support and typescript support
1. Add an implicit `import Vue from 'vue'` and `new Vue(...)` around the
default exported object literal. This provides a lot better intellisense
than just a bare object literal because the Typescript language server
knows that the object will be used by Vue.
2. Resolve other .vue files so that `import other from './other.vue'`
provides correct completions.
3. If lang='typescript' is specified, put the TS language service in TS
mode: all errors are reported and completions include only symbols that
are known to be correct.
4. Also fix a bug in htmlServerMain so that errors are actually reported.
@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 16, 2017

@mhegazy you were interested in this PR, you said.

@octref

This comment has been minimized.

Member

octref commented Mar 16, 2017

Thanks a lot for the PR!

Just trying it out on this repo: https://github.com/vuejs/vue-hackernews-2.0, but IntelliSense didn't work:

  • Vue. doesn't prompt Vue.extend in *.vue files (works in js files). Thought this would work with the implicit addition of import Vue from 'vue'
  • Newly opened *.vue files don't have IntelliSense and always show "loading"
    image
  • As for new Vue(...), if I have Item.vue with a prop count, and in another vue file I import Item.vue, is it that
    • Item. is supposed to prompt all values on the new Vue(...) object?
    • Item.props. should prompt count?

Just a rough look at the code, it seems I need a properly configured jsconfig/tsconfig.

Would you mind adding appropriate config for a sample vue project so it would work as you intended? Either https://github.com/vuejs/vue-hackernews-2.0 or something from here https://github.com/vuejs/vue/tree/dev/examples

@octref

This comment has been minimized.

Member

octref commented Mar 16, 2017

Oh and in vetur's language server I'm using TS 2.2.1. Is is I should install a dev build or build from typescript's master and link locally?

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 16, 2017

I'll test with vue-hackernews-2.0, in particular without a tsconfig. I thought the change would work without a tsconfig, but I didn't test that way, so I'm not surprised that it doesn't.

  • I don't think you need any special version of TS. I'll double check though.
  • "Loading..." usually means the language service asserted and hit the debugger statement we have there. I'll check it out. (This is probably the cause of no completion for Vue.extends too.)
  • Is this the code you're thinking of?:
// @filename:Item.vue
export default {
  props: ['count'],
  data: {
    d1: 12
  }
  // etc ...
}

// @filename:SomeComponent.vue
//          (or somethingElse.js)
import Item from './Item.vue'
Item./**/ // should see d1, $data, $parent, etc
Item.props./**/ // should see count

The short answer is yes, those should work. But vue.d.ts doesn't support a lot of these things yet. @DanielRosenwasser is working on improvements to vue.d.ts that rely on Typescript 2.3 features. Once 2.3 ships with VS Code, the experience will get a lot better.

Unfortunately, better this inference relies on 2.3 as well. Even without 2.3, though, this change makes module resolution work, and will pick up any other improvements to vue.d.ts that happen in the meantime.

@octref

This comment has been minimized.

Member

octref commented Mar 16, 2017

I tested with lodash and @types/lodash, module resolution works great. Thanks!

Is this the code you're thinking of?

Something like that. I'm wondering if there is a way to get the Vue wrapper object's type information on Item.vue at LS side so I can use it to power html suggestion. Item./**/ IntelliSense in JS region wouldn't be helpful. But this kind of IntelliSense in html template region would be godly:

<i/**/ // Prompt item

<item :/**/> // Prompt count and other props
@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 16, 2017

Yes, from typescript's perspective the object from Item.vue is already a Vue object because it was called from new Vue(...). However, you would have to make the typescriptMode.parse code much smarter to use that in HTML. Currently it just replaces things outside <script> with spaces. If it replaced non-standard HTML elements with valid typescript, which would each have to be smaller than the original HTML so they didn't overflow the original, it could provide completions on that typescript.

I think the Angular guys have done or were thinking about doing something like that for Angular 2. I don't know Angular though, so I'm not sure of the details.

You might also do some other clever thing with a virtual TextDocument for each HTML element, which would make the host code in javascriptMode much more complicated. And I don't know whether you could do anything like validation, only completions, because the offsets from the language service would likely be quite wrong.

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 16, 2017

One update and one question:

  1. I confirmed that this works with Typescript on master and 2.2. So both typescript@next and the current Typescript should be fine.
  2. Should the inserted call be Vue.extends instead of new Vue?
@octref

This comment has been minimized.

Member

octref commented Mar 16, 2017

Should the inserted call be Vue.extends instead of new Vue?

I think new Vue.

// (the span of the construct call is the same as the object literal)
const objectLiteral = (exportDefaultObject as ts.ExportAssignment).expression as ts.ObjectLiteralExpression;
const o = <T extends ts.TextRange>(n: T) => ts.setTextRange(n, objectLiteral);
const vue = ts.setTextRange(ts.createIdentifier('Vue'), { pos: objectLiteral.pos, end: objectLiteral.pos + 1 });

This comment has been minimized.

@octref

octref Mar 16, 2017

Member

Would you mind giving o and n more descriptive names? I'm not quite familiar with TS compilers but would want to fiddle with this part of code.

@octref

This comment has been minimized.

Member

octref commented Mar 17, 2017

I'll merge once you fix the one comment.

Meanwhile thanks for giving me a starting point -- I guess for now I'll just use TS to parse the object literal's AST, find out props / methods / data manually to provide some IntelliSense.

image

sandersn added some commits Mar 17, 2017

Improve names and explanation of range-setting code
It's required to make sure that getXXXAtLocation works correctly,
because if the ranges are missing from the synthetic nodes, the language
service asserts, and if the ranges are wrong, the language service's AST
search gets confused.
@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 17, 2017

I improved the naming, structure and documentation of the AST creation utilities. Let me know if you have other questions about creating or altering ASTs.

I'm still investigating the failure to resolve 'vue'. It works fine when I use './node_modules/vue/types/index' instead. My test project may have incorrect settings in tsconfig.

return vueDocument.getEmbeddedDocument('typescript');
}
return vueDocument.getEmbeddedDocument('javascript');
});

let compilerOptions: ts.CompilerOptions = { allowNonTsExtensions: true, allowJs: true, lib: ['lib.es6.d.ts'], target: ts.ScriptTarget.Latest, moduleResolution: ts.ModuleResolutionKind.Classic };

This comment has been minimized.

@mhegazy

mhegazy Mar 17, 2017

For this i would defere to ts.getDefaultCompilerOptions and just set:

let compilerOptions = ts.getDefaultCompilerOptions();
if (docs[fileName].languageId !== 'typescript') {
     copilerOptions.allowJs = true
}
compilerOptions.allowNonTsExtensions = true;

This comment has been minimized.

@mhegazy

mhegazy Mar 17, 2017

I take that back..

{
   "allowJs": docs[fileName].languageId !== 'typescript',
   "lib" : ["dom", "esNext"],
   "target" : ["esNext"],
   "module": ts.ModuleKind.CommonJs
}

This comment has been minimized.

@sandersn

sandersn Mar 17, 2017

Contributor

"esnext" didn't ship as a lib target in 2.2, so I'll use "es2017" for now.
(It was committed Dec 30, but I guess it was part of a PR that got merged later.)

compilerOptions,
configFile,
undefined,
[{ extension: 'vue', isMixedContent: true }]).fileNames;

This comment has been minimized.

@mhegazy

mhegazy Mar 17, 2017

should also set compilerOptions to the options from the file. we also need to set compilerOptions.allowNonTsExtensions = true; all the time to allow .vue files.

This comment has been minimized.

@sandersn

sandersn Mar 17, 2017

Contributor

done

}

export function createUpdater() {
const clssf = ts.createLanguageServiceSourceFile;

This comment has been minimized.

@mhegazy

mhegazy Mar 17, 2017

these should be added to the LanguageServiceHost API in TS 2.3, I would also add a comment here with that to allow removing this global function hijacking.

This comment has been minimized.

@sandersn

sandersn Mar 17, 2017

Contributor

added comment where the hijacking happens. This part of the code may remain the same since it's basically the same way that the plugin API allows users to wrap the language service.

}
return '1'; // default lib an jquery.d.ts are static
else {
return (ts as any).getScriptKindFromFileName(fileName);

This comment has been minimized.

@mhegazy

mhegazy Mar 17, 2017

We need to expose this on the API for TS 2.3, also add a comment allowing removal of this cast.

This comment has been minimized.

@sandersn

sandersn Mar 17, 2017

Contributor

added comment. I'll send out a PR for the API change after this.


let compilerOptions: ts.CompilerOptions = { allowNonTsExtensions: true, allowJs: true, lib: ['lib.es6.d.ts'], target: ts.ScriptTarget.Latest, moduleResolution: ts.ModuleResolutionKind.Classic };
let currentTextDocument: TextDocument;
let scriptFileVersion: number = 0;
let versions: ts.MapLike<number> = {};

This comment has been minimized.

@mhegazy

mhegazy Mar 17, 2017

you can use ES6 Map

This comment has been minimized.

@sandersn

sandersn Mar 17, 2017

Contributor

done

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 17, 2017

All right, I addressed all of @mhegazy's comments. I think it's ready to go in but I'll spend time testing without a tsconfig now to make sure.

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 17, 2017

The failure to resolve import Vue from 'vue' was due to missing allowSyntheticDefaultImports: true and having moduleResolution: ts.ModuleResolutionKind.Classic. It needs to be NodeJs.

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 17, 2017

OK, vue-hackernews-2.0 is now working and gives nice (?) errors when you switch to lang="typescript". Actually the vue.d.ts needs some updates to make this mode actually nice because it issues some bogus errors.

For both javascript and typescript, the completions are pretty nice, though.

I'm chasing down some bugs to do with changing the language back and forth between javascript and typescript. I think this is ready to merge as-is though. I'll send another PR if/when I track down the language-changing bug.

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 17, 2017

Actually, the language-change bug repros when opening or closing files. Probably you should wait to merge this until I track it down.

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 17, 2017

If you open a .vue file with lang='typescript' and then open one without, then the newly opened file gets initially set as typescript, even though it shouldn't. But then when you interact with the language service, it resets the type to javascript, which hits the assert.

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 17, 2017

Hm, so this is not an easy fix. The problem is that javascriptmode.ts doesn't know the languageId of files that are not opened yet, but the language service still wants to know whether they contain JavaScript or typescript. There are two basic approaches to fix this:

  1. Start with an initial guess. Right now this causes an assert if the guess is wrong because the file type switches in the language service.
  2. Add files to the language model cache as the language service requests them. This bloats the cache, but arguably it makes vetur itself more like a real language service.

I think (2) is the right solution, but it's more work. I'll do what I can but my next 48 hours are pretty busy with non-work things, so it may be Monday before I have something working.

Register unseen vue files when TSLS requests them
The Typescript language service references files that are not open for
editing, and thus don't have entries in the language model cache. Since
they don't have entries, there is no way to know whether their script
block is Javascript or Typescript.

The fix is to load these files and register them with the language model cache.
@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 18, 2017

Turns out the fix is pretty small after all: calling jsDocuments.get on a new TextDocument implicitly registers it with the cache.

Unfortunately, since I was working from my laptop, a Windows machine, I discovered that path handling is completely broken on Windows. url.parse(filename).path is not a complete solution for Windows filenames, which are rooted at c:/ or some other drive. There's an extra preceding / and (at least) the ':' is encoded as '%3A'. This has to be a solved problem within VS Code, so I'm going to find out what the existing solution is.

@octref

This comment has been minimized.

Member

octref commented Mar 20, 2017

Are you looking for this?

https://github.com/Microsoft/vscode/blob/master/src/vs/base/common/uri.ts#L126-L145

I'm not against including that as part of the source. I might need it too.

However, overall I think it's better to have a fine-tuned default ts/js config internally, as I imagine most Vue users wouldn't bother to specify js/ts config. (Though reading tsconfig if it's present doesn't hurt).

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 20, 2017

Yes, I am also looking at it right now and wondering what the public version of it is. I asked @mjbvz as well so I should know pretty soon.

Unfortunately, the host also calls ts.sys.readFile several times so that the language service is able to analyse files that are not open in the editor. It's not just for tsconfig support. 😿

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 20, 2017

Looks this package is the right answer: https://github.com/Microsoft/vscode-uri

sandersn added some commits Mar 20, 2017

Correctly normalise slashes on Windows
TS server and VS Code normalise them in different ways. Standardise by
passing everything through vscode-uri
@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 20, 2017

Ok, windows paths should be working now. Both URL encoding and slash normalisation had to be fixed. I'll check once more on my Linux box to make sure I didn't break anything there, but I think it's ready to merge now.

@sandersn

This comment has been minimized.

Contributor

sandersn commented Mar 20, 2017

All right, everything looks fine on Linux as well, so this PR is good to go.

@octref octref merged commit c6bbd43 into vuejs:master Mar 21, 2017

@octref

This comment has been minimized.

Member

octref commented Mar 21, 2017

Thanks a lot! Will include this in 0.6 release.

@octref

This comment has been minimized.

Member

octref commented May 8, 2017

@sandersn Do you know @DanielRosenwasser's progress of the vue.d.ts update that would use contextualThis to transform the wrapped new Vue() object?

Or is is I should just start writing a dts lib that would transform a new Vue() object into an extended form which would have all props, methods and computed bound to it? Then I can put it in here to bind the new object to this: https://github.com/octref/vetur/blob/master/server/src/modes/typescriptMode.ts#L68-L74

@DanielRosenwasser

This comment has been minimized.

DanielRosenwasser commented May 8, 2017

Right now you can try it all out on https://github.com/DanielRosenwasser/typescript-vue-todomvc, but I need to chat with @yyx990803 about it, but I think the earliest we could get it out is Vue 2.4.

By the way, right now TypeScript users can't rely on contextually typing the object literal because at compile time, there is no call to Vue.extend (or new Vue). So while you'll get no errors in JS scenarios, TypeScript will spit out errors when you run Webpack.

In short, TypeScript users still need to wrap their components in calls to Vue.extend which I think is fine.

@octref

This comment has been minimized.

Member

octref commented May 8, 2017

Right now you can try it all out on https://github.com/DanielRosenwasser/typescript-vue-todomvc

Working great for me, thanks!

TypeScript users still need to wrap their components in calls to Vue.extend

That's fine. I'm wondering if it's possible to modify the source at compile time using a TS loader for webpack.
I didn't delve into vue's internal yet, but I guess Vue.extend is the same thing as class App extends Vue? People are already writing

export default class App extends Vue {
  ...
}

with https://github.com/vuejs/vue-class-component so I guess that's fine.

Also wondering what's your thoughts on typed props. I see some work towards that in vue.d.ts.

Meanwhile I'll start building support for dynamic IntelliSense for template completion in #145. That seems doable without contextual typing anyway.

@DanielRosenwasser

This comment has been minimized.

DanielRosenwasser commented May 8, 2017

Also wondering what's your thoughts on typed props

Props is pretty crazy to type because it might come in as an array of strings, or it might come in as an object which you can infer from. Ideally I could have a PropNames extends string = never and Props, and have props: SomeMappedType<Props> | PropNames[] (or something like that), but it started getting complicated.

I'll try it out after next week. Right now we're getting some stuff together for our Build conference.

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