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

Type checking for template expressions #681

Merged
merged 55 commits into from Apr 15, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
e98740d
Move script mode integration test
ktsn Nov 14, 2017
e4b1549
Provide diagnostics for template by using type info
ktsn Nov 26, 2017
dc9d8c1
[wip] template diagnostics
ktsn Dec 12, 2017
912b134
Use vue-eslint-parser
ktsn Dec 15, 2017
a68726e
transform vue-eslint-parser ast to ts ast
ktsn Jan 7, 2018
a66b78c
format codes
ktsn Jan 7, 2018
a10d492
template diagnostics provide correct error positions
ktsn Jan 7, 2018
6865c30
Inject this expression for template identifiers
ktsn Feb 3, 2018
34a58dc
check v-for expression
ktsn Feb 4, 2018
3b852e4
remove global scope test of injectThis
ktsn Feb 4, 2018
de2e859
fix the position of v-for expression
ktsn Feb 4, 2018
c6a38a0
rewrite template render function
ktsn Feb 4, 2018
b90f96a
Avoid parsing error of script block
ktsn Feb 5, 2018
1f02005
Handle object literal expression properly
ktsn Feb 5, 2018
deaa8a7
Move template type checking fixtures
ktsn Feb 5, 2018
68b8200
Process template code as JS to avoid unnecessary errors
ktsn Feb 5, 2018
6fc1e52
Handle v-on statement properly
ktsn Feb 5, 2018
60562ef
Extract common logic of template checking test
ktsn Feb 5, 2018
3138243
Refactoring transformTemplate
ktsn Feb 5, 2018
7696040
Remove unused @types/estree
ktsn Feb 5, 2018
b7a805b
Use component constructor directly in generated template ts code
ktsn Feb 5, 2018
a8ef51e
Bump Vue typings for testing
ktsn Feb 5, 2018
0c97e75
Remove unused packages
ktsn Feb 5, 2018
1642733
Rename internal template helpers
ktsn Feb 8, 2018
ce43c75
Simplify v-on transformation
ktsn Feb 8, 2018
55191cb
Merge branch 'master' into template-type-checking
ktsn Feb 23, 2018
bd98089
Merge remote-tracking branch 'upstream/master' into template-type-che…
ktsn Aug 4, 2018
9f5304d
Merge remote-tracking branch 'upstream/master' into template-type-che…
ktsn Dec 10, 2018
f2f3236
support literal iteration of v-for
ktsn Dec 11, 2018
bca4ad4
add a flag to control template type check
ktsn Dec 11, 2018
5d6d62f
inject 'this' to expressions in template expression
ktsn Dec 11, 2018
0fd53f0
check array literal expression
ktsn Dec 21, 2018
0542c1a
bump vue-eslint-parser
ktsn Dec 21, 2018
653bf3d
enable noImplicitAny on template region
ktsn Dec 21, 2018
aabba0f
avoid duplicated identifier error when no script block
ktsn Dec 30, 2018
31d88bf
allow to put normal class/style and v-bind ones on the same element
ktsn Dec 30, 2018
bec63eb
check directive expression
ktsn Dec 30, 2018
71fd337
add test case of directive check
ktsn Dec 30, 2018
4e73004
Merge remote-tracking branch 'upstream/master' into template-type-che…
ktsn Jan 3, 2019
b8be711
inject this value to ElementAccessExpression
ktsn Jan 14, 2019
889d1ce
inject this to computed property name of object literal
ktsn Jan 14, 2019
39c807b
Various improvements for template type checking
octref Apr 10, 2019
30ddc84
Merge remote-tracking branch 'origin/ktsn-types' into HEAD
octref Apr 10, 2019
e26d5d9
Small cleanup
octref Apr 10, 2019
79dc034
:lipstick:
octref Apr 10, 2019
ef7978a
Fix some failing tests
octref Apr 10, 2019
c143dfa
handle VExpressionContainer of dynamic directive argument
ktsn Apr 13, 2019
d0713d1
Support dynamic argument of directive
ktsn Apr 13, 2019
eff560a
Fix broken v-on template diagnostic test
ktsn Apr 13, 2019
f428765
Simplify special attribute check
ktsn Apr 13, 2019
2923e08
Skip v-slot check to avoid false-positive error
ktsn Apr 13, 2019
96797c0
Fix typo
ktsn Apr 13, 2019
de998c6
Merge remote-tracking branch 'origin/master' into template-type-checking
octref Apr 13, 2019
532074a
Update comment
ktsn Apr 14, 2019
ab4a5cc
Fix createLiteral argument to correct value
ktsn Apr 14, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 3 additions & 4 deletions client/vueMain.ts
Expand Up @@ -22,10 +22,9 @@ export function activate(context: vscode.ExtensionContext) {

context.subscriptions.push(
vscode.commands.registerCommand('vetur.chooseTypeScriptRefactoring', (args: any) => {
client.sendRequest<vscode.Command | undefined>('requestCodeActionEdits', args)
.then(command =>
command && vscode.commands.executeCommand(command.command, ...command.arguments!)
);
client
.sendRequest<vscode.Command | undefined>('requestCodeActionEdits', args)
.then(command => command && vscode.commands.executeCommand(command.command, ...command.arguments!));
})
);

Expand Down
5 changes: 5 additions & 0 deletions package.json
Expand Up @@ -412,6 +412,11 @@
"vetur.dev.vlsPath": {
"type": "string",
"description": "Path to VLS for Vetur developers. There are two ways of using it. \n\n1. Clone vuejs/vetur from GitHub, build it and point it to the ABSOLUTE path of `/server`.\n2. `yarn global add vue-language-server` and point Vetur to the installed location (`yarn global dir` + node_modules/vue-language-server)"
},
"vetur.experimental.templateTypeCheck": {
"type": "boolean",
"default": true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can enable it by default to get more feedback.

"description": "Type-check interpolation expressions in <template> region"
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions server/package.json
Expand Up @@ -44,11 +44,14 @@
"vscode-languageserver": "^5.3.0-next.4",
"vscode-languageserver-types": "^3.15.0-next.1",
"vscode-uri": "^1.0.1",
"vue-eslint-parser": "^6.0.3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using v6 now. There are a few changes in the AST format. Take a look at the transformer for Todo comments.

"vue-onsenui-helper-json": "^1.0.2",
"vuetify-helper-json": "^1.0.0"
},
"devDependencies": {
"@types/eslint": "^4.16.5",
"@types/eslint-scope": "^3.7.0",
"@types/eslint-visitor-keys": "^1.0.0",
"@types/glob": "^7.1.0",
"@types/js-beautify": "^1.8.0",
"@types/lodash": "^4.14.118",
Expand Down
33 changes: 28 additions & 5 deletions server/src/modes/script/bridge.ts
@@ -1,21 +1,44 @@
// this bridge file will be injected into TypeScript service
import { renderHelperName, componentHelperName, iterationHelperName, listenerHelperName } from './transformTemplate';

// This bridge file will be injected into TypeScript language service
// it enable type checking and completion, yet still preserve precise option type

export const moduleName = 'vue-editor-bridge';

export const fileName = 'vue-temp/vue-editor-bridge.ts';

export const oldContent = `
const renderHelpers = `
export declare const ${renderHelperName}: {
<T>(Component: (new (...args: any[]) => T), fn: (this: T) => any): any;
};
export declare const ${componentHelperName}: {
(tag: string, data: any, children: any[]): any;
};
export declare const ${iterationHelperName}: {
<T>(list: T[], fn: (value: T, index: number) => any): any;
<T>(obj: { [key: string]: T }, fn: (value: T, key: string, index: number) => any): any;
(num: number, fn: (value: number) => any): any;
<T>(obj: object, fn: (value: any, key: string, index: number) => any): any;
};
export declare const ${listenerHelperName}: {
<T>(vm: T, listener: (this: T, ...args: any[]) => any): any;
};
`;

export const oldContent =
`
import Vue from 'vue';
export interface GeneralOption extends Vue.ComponentOptions<Vue> {
[key: string]: any;
}
export default function bridge<T>(t: T & GeneralOption): T {
return t;
}`;
}
` + renderHelpers;

export const content = `
export const content =
`
import Vue from 'vue';
const func = Vue.extend;
export default func;
`;
` + renderHelpers;
42 changes: 40 additions & 2 deletions server/src/modes/script/javascript.ts
Expand Up @@ -102,18 +102,23 @@ export async function getJavascriptMode(
},

doValidation(doc: TextDocument): Diagnostic[] {
const templateDiags = getTemplateDiagnostics();
const scriptDiags = getScriptDiagnostics();
return [...templateDiags, ...scriptDiags];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided to keep the diagnostics calculation all in JS mode.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octref I realized this might cause an edge case. When we do not have <script> block in SFC, template diagnostic won't work. I'm not sure how we should include script language mode in that case 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ktsn When you don't have <script> block, what template diagnostic do you want?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly detecting some typos. For example, sometimes I have some typo in template expression like:

<template>
  <ChildComp @click="$('click')" /> <!-- <= typo of $emit('click') -->
</template>

Another example is when we use scoped slot component (scoped slot variables not handled currently though):

<template>
  <apollo-query :query="..." v-slot="{ isLoading, result }">
    <-- typo of v-if="isLoading" -->
    <div v-if="loading">Loading...</div>

    <div v-else-if="result.data">{{ result.data }}</div>
  </apollo-query>
</template>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll open a new issue for this.

Let's keep these two things separate:

  • Where errors are generated — Should be in JS mode
  • When should diagnostics be re-generated — For <template> part, if there's a change in template interpolation, it should re-trigger JS mode diagnostics


function getScriptDiagnostics(): Diagnostic[] {
const { scriptDoc, service } = updateCurrentTextDocument(doc);
if (!languageServiceIncludesFile(service, doc.uri)) {
return [];
}

const fileFsPath = getFileFsPath(doc.uri);
const diagnostics = [
const rawScriptDiagnostics = [
...service.getSyntacticDiagnostics(fileFsPath),
...service.getSemanticDiagnostics(fileFsPath)
];

return diagnostics.map(diag => {
return rawScriptDiagnostics.map(diag => {
const tags: DiagnosticTag[] = [];

if (diag.reportsUnnecessary) {
Expand All @@ -131,6 +136,39 @@ export async function getJavascriptMode(
source: 'Vetur'
};
});
}

function getTemplateDiagnostics(): Diagnostic[] {
const enabledTemplateValidation = config.vetur.experimental.templateTypeCheck;
if (!enabledTemplateValidation) {
return [];
}

// Add suffix to process this doc as vue template.
const templateDoc = TextDocument.create(doc.uri + '.template', doc.languageId, doc.version, doc.getText());

const { templateService } = updateCurrentTextDocument(templateDoc);
if (!languageServiceIncludesFile(templateService, templateDoc.uri)) {
return [];
}

const templateFileFsPath = getFileFsPath(templateDoc.uri);
// We don't need syntactic diagnostics because
// compiled template is always valid JavaScript syntax.
const rawTemplateDiagnostics = templateService.getSemanticDiagnostics(templateFileFsPath);

return rawTemplateDiagnostics.map(diag => {
// syntactic/semantic diagnostic always has start and length
// so we can safely cast diag to TextSpan
return {
range: convertRange(templateDoc, diag as ts.TextSpan),
severity: DiagnosticSeverity.Error,
message: ts.flattenDiagnosticMessageText(diag.messageText, '\n'),
code: diag.code,
source: 'Vetur'
};
});
}
},
doComplete(doc: TextDocument, position: Position): CompletionList {
const { scriptDoc, service } = updateCurrentTextDocument(doc);
Expand Down
165 changes: 152 additions & 13 deletions server/src/modes/script/preprocess.ts
@@ -1,32 +1,85 @@
import * as ts from 'typescript';
import * as path from 'path';
import { parse } from 'vue-eslint-parser';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can implement parsing in VLS at all, so no additional dependency/script is needed. Actually template completion needs it too.

But I don't have time to implement Vue specific elements 😞 . For now eslint-parser is the only option.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I actually tried to extend the parser in VLS on the first time but I ended up using vue-eslint-parser because I would like to focus finishing essential implementation of template type checking at first.
In that case, I can work on the VSL template parser after this PR is finished 🙂


import { getVueDocumentRegions } from '../../embeddedSupport/embeddedSupport';
import { TextDocument } from 'vscode-languageserver-types';
import { T_TypeScript } from '../../services/dependencyService';
import {
getTemplateTransformFunctions,
componentHelperName,
iterationHelperName,
renderHelperName,
listenerHelperName
} from './transformTemplate';
import { isVirtualVueTemplateFile } from './serviceHost';

export function isVue(filename: string): boolean {
return path.extname(filename) === '.vue';
}

export function parseVue(text: string): string {
export function parseVueScript(text: string): string {
const doc = TextDocument.create('test://test/test.vue', 'vue', 0, text);
const regions = getVueDocumentRegions(doc);
const script = regions.getSingleTypeDocument('script');
return script.getText() || 'export default {};';
}

function parseVueTemplate(text: string): string {
const doc = TextDocument.create('test://test/test.vue', 'vue', 0, text);
const regions = getVueDocumentRegions(doc);
const template = regions.getSingleTypeDocument('template');

if (template.languageId !== 'vue-html') {
return '';
}
const rawText = template.getText();
// skip checking on empty template
if (rawText.replace(/\s/g, '') === '') {
return '';
}
return rawText.replace(/^\s*\n/, '<template>\n').replace(/\s*\n$/, '\n</template>');
}

export function createUpdater(tsModule: T_TypeScript) {
const clssf = tsModule.createLanguageServiceSourceFile;
const ulssf = tsModule.updateLanguageServiceSourceFile;
const scriptKindTracker = new WeakMap<ts.SourceFile, ts.ScriptKind | undefined>();
const modificationTracker = new WeakSet<ts.SourceFile>();

function isTSLike(scriptKind: ts.ScriptKind | undefined) {
return scriptKind === ts.ScriptKind.TS || scriptKind === ts.ScriptKind.TSX;
return scriptKind === tsModule.ScriptKind.TS || scriptKind === tsModule.ScriptKind.TSX;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vetur now uses TS from workspace, or fallbacks to bundled TS. That's why you need to use tsModule instead of ts in all cases. Using types from ts is fine.

I'll do the conversion for transformTemplate too.

}

return {
createLanguageServiceSourceFile(
function modifySourceFile(
fileName: string,
sourceFile: ts.SourceFile,
scriptSnapshot: ts.IScriptSnapshot,
version: string,
scriptKind?: ts.ScriptKind
): void {
if (modificationTracker.has(sourceFile)) {
return;
}

if (isVue(fileName) && !isTSLike(scriptKind)) {
modifyVueScript(tsModule, sourceFile);
modificationTracker.add(sourceFile);
return;
}

if (isVirtualVueTemplateFile(fileName)) {
// TODO: share the logic of transforming the code into AST
// with the template mode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ktsn Can you clarify what's your plan for this one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To use Vetur's template parser instead of vue-eslint-parser. I guess I'll work on it after this PR.

const code = parseVueTemplate(scriptSnapshot.getText(0, scriptSnapshot.getLength()));
const program = parse(code, { sourceType: 'module' });
const tsCode = getTemplateTransformFunctions(tsModule).transformTemplate(program, code);
injectVueTemplate(tsModule, sourceFile, tsCode);
modificationTracker.add(sourceFile);
}
}

function createLanguageServiceSourceFile(
fileName: string,
scriptSnapshot: ts.IScriptSnapshot,
scriptTarget: ts.ScriptTarget,
Expand All @@ -36,12 +89,11 @@ export function createUpdater(tsModule: T_TypeScript) {
): ts.SourceFile {
const sourceFile = clssf(fileName, scriptSnapshot, scriptTarget, version, setNodeParents, scriptKind);
scriptKindTracker.set(sourceFile, scriptKind);
if (isVue(fileName) && !isTSLike(scriptKind)) {
modifyVueSource(tsModule, sourceFile);
}
modifySourceFile(fileName, sourceFile, scriptSnapshot, version, scriptKind);
return sourceFile;
},
updateLanguageServiceSourceFile(
}

function updateLanguageServiceSourceFile(
sourceFile: ts.SourceFile,
scriptSnapshot: ts.IScriptSnapshot,
version: string,
Expand All @@ -50,15 +102,17 @@ export function createUpdater(tsModule: T_TypeScript) {
): ts.SourceFile {
const scriptKind = scriptKindTracker.get(sourceFile);
sourceFile = ulssf(sourceFile, scriptSnapshot, version, textChangeRange, aggressiveChecks);
if (isVue(sourceFile.fileName) && !isTSLike(scriptKind)) {
modifyVueSource(tsModule, sourceFile);
}
modifySourceFile(sourceFile.fileName, sourceFile, scriptSnapshot, version, scriptKind);
return sourceFile;
}

return {
createLanguageServiceSourceFile,
updateLanguageServiceSourceFile
};
}

function modifyVueSource(tsModule: T_TypeScript, sourceFile: ts.SourceFile): void {
function modifyVueScript(tsModule: T_TypeScript, sourceFile: ts.SourceFile): void {
const exportDefaultObject = sourceFile.statements.find(
st =>
st.kind === tsModule.SyntaxKind.ExportAssignment &&
Expand Down Expand Up @@ -94,6 +148,91 @@ function modifyVueSource(tsModule: T_TypeScript, sourceFile: ts.SourceFile): voi
}
}

/**
* Wrap render function with component options in the script block
* to validate its types
*/
function injectVueTemplate(tsModule: T_TypeScript, sourceFile: ts.SourceFile, renderBlock: ts.Expression[]): void {
// add import statement for corresponding Vue file
// so that we acquire the component type from it.
const setZeroPos = getWrapperRangeSetter(tsModule, { pos: 0, end: 0 });
const vueFilePath = './' + path.basename(sourceFile.fileName.slice(0, -9));
const componentImport = setZeroPos(
tsModule.createImportDeclaration(
undefined,
undefined,
setZeroPos(tsModule.createImportClause(setZeroPos(tsModule.createIdentifier('__Component')), undefined)),
setZeroPos(tsModule.createLiteral(vueFilePath))
)
);

// import helper type to handle Vue's private methods
const helperImport = setZeroPos(
tsModule.createImportDeclaration(
undefined,
undefined,
setZeroPos(
tsModule.createImportClause(
undefined,
setZeroPos(
tsModule.createNamedImports([
setZeroPos(
tsModule.createImportSpecifier(undefined, setZeroPos(tsModule.createIdentifier(renderHelperName)))
),
setZeroPos(
tsModule.createImportSpecifier(undefined, setZeroPos(tsModule.createIdentifier(componentHelperName)))
),
setZeroPos(
tsModule.createImportSpecifier(undefined, setZeroPos(tsModule.createIdentifier(iterationHelperName)))
),
setZeroPos(
tsModule.createImportSpecifier(undefined, setZeroPos(tsModule.createIdentifier(listenerHelperName)))
)
])
)
)
),
setZeroPos(tsModule.createLiteral('vue-editor-bridge'))
)
);

// wrap render code with a function decralation
// with `this` type of component.
const setRenderPos = getWrapperRangeSetter(tsModule, sourceFile);
const statements = renderBlock.map(exp => tsModule.createStatement(exp));
const renderElement = setRenderPos(
tsModule.createStatement(
setRenderPos(
tsModule.createCall(setRenderPos(tsModule.createIdentifier(renderHelperName)), undefined, [
// Reference to the component
setRenderPos(tsModule.createIdentifier('__Component')),

// A function simulating the render function
setRenderPos(
tsModule.createFunctionExpression(
undefined,
undefined,
undefined,
undefined,
[],
undefined,
setRenderPos(tsModule.createBlock(statements))
)
)
])
)
)
);

// replace the original statements with wrapped code.
sourceFile.statements = setRenderPos(tsModule.createNodeArray([componentImport, helperImport, renderElement]));

// Update external module indicator to the transformed template node,
// otherwise symbols in this template (e.g. __Component) will be put
// into global namespace and it causes duplicated identifier error.
(sourceFile as any).externalModuleIndicator = componentImport;
}

/** Create a function that calls setTextRange on synthetic wrapper nodes that need a valid range */
function getWrapperRangeSetter(
tsModule: T_TypeScript,
Expand Down