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

(chore) bumps typescript to 4.7 #1496

Merged
merged 4 commits into from May 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -16,7 +16,7 @@
"lint": "prettier --check . && eslint \"packages/**/*.{ts,js}\""
},
"dependencies": {
"typescript": "^4.6.2"
"typescript": "^4.7.2"
},
"devDependencies": {
"@sveltejs/eslint-config": "github:sveltejs/eslint-config#v5.2.0",
Expand Down
@@ -1,13 +1,14 @@
import { ComponentEvents } from 'svelte2tsx';
import ts from 'typescript';
import { isNotNullOrUndefined } from '../../utils';
import { flatten, isNotNullOrUndefined } from '../../utils';
import { findContainingNode } from './features/utils';

export type ComponentPartInfo = ReturnType<ComponentEvents['getAll']>;

export interface ComponentInfoProvider {
getEvents(): ComponentPartInfo;
getSlotLets(slot?: string): ComponentPartInfo;
getProps(propName: string): ts.CompletionEntry[];
}

export class JsOrTsComponentInfoProvider implements ComponentInfoProvider {
Expand Down Expand Up @@ -44,6 +45,61 @@ export class JsOrTsComponentInfoProvider implements ComponentInfoProvider {
return this.mapPropertiesOfType(slotLetsType);
}

getProps(propName: string): ts.CompletionEntry[] {
const props = this.getType('$$prop_def');
if (!props) {
return [];
}

const prop = props.getProperties().find((prop) => prop.name === propName);
if (!prop?.valueDeclaration) {
return [];
}

const propDef = this.typeChecker.getTypeOfSymbolAtLocation(prop, prop.valueDeclaration);

if (!propDef.isUnion()) {
return [];
}

const types = flatten(propDef.types.map((type) => this.getStringLiteralTypes(type)));

// adopted from https://github.com/microsoft/TypeScript/blob/0921eac6dc9eba0be6319dff10b85d60c90155ea/src/services/stringCompletions.ts#L61
return types.map((v) => ({
name: v.value,
kindModifiers: ts.ScriptElementKindModifier.none,
kind: ts.ScriptElementKind.string,
sortText: /**LocationPriority: */ '11'
}));
}

/**
* adopted from https://github.com/microsoft/TypeScript/blob/0921eac6dc9eba0be6319dff10b85d60c90155ea/src/services/stringCompletions.ts#L310
*/
private getStringLiteralTypes(
type: ts.Type | undefined,
uniques = new Set<string>()
): ts.StringLiteralType[] {
if (!type) {
return [];
}

type = type.isTypeParameter() ? type.getConstraint() || type : type;

if (type.isUnion()) {
return flatten(type.types.map((t) => this.getStringLiteralTypes(t, uniques)));
}

if (
type.isStringLiteral() &&
!(type.flags & ts.TypeFlags.EnumLiteral) &&
!uniques.has(type.value)
) {
return [type];
}
return [];
}

private getType(classProperty: string) {
const symbol = this.classType.getProperty(classProperty);
if (!symbol?.valueDeclaration) {
Expand Down
Expand Up @@ -50,7 +50,7 @@ import {
} from '../interfaces';
import { CodeActionsProviderImpl } from './features/CodeActionsProvider';
import {
CompletionEntryWithIdentifer,
CompletionEntryWithIdentifier,
CompletionsProviderImpl
} from './features/CompletionProvider';
import { DiagnosticsProviderImpl } from './features/DiagnosticsProvider';
Expand Down Expand Up @@ -91,7 +91,7 @@ export class TypeScriptPlugin
ImplementationProvider,
TypeDefinitionProvider,
OnWatchFileChanges,
CompletionsProvider<CompletionEntryWithIdentifer>,
CompletionsProvider<CompletionEntryWithIdentifier>,
UpdateTsOrJsFile
{
__name = 'ts';
Expand Down Expand Up @@ -273,7 +273,7 @@ export class TypeScriptPlugin
position: Position,
completionContext?: CompletionContext,
cancellationToken?: CancellationToken
): Promise<AppCompletionList<CompletionEntryWithIdentifer> | null> {
): Promise<AppCompletionList<CompletionEntryWithIdentifier> | null> {
if (!this.featureEnabled('completions')) {
return null;
}
Expand Down Expand Up @@ -303,9 +303,9 @@ export class TypeScriptPlugin

async resolveCompletion(
document: Document,
completionItem: AppCompletionItem<CompletionEntryWithIdentifer>,
completionItem: AppCompletionItem<CompletionEntryWithIdentifier>,
cancellationToken?: CancellationToken
): Promise<AppCompletionItem<CompletionEntryWithIdentifer>> {
): Promise<AppCompletionItem<CompletionEntryWithIdentifier>> {
return this.completionProvider.resolveCompletion(
document,
completionItem,
Expand Down
Expand Up @@ -142,9 +142,9 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
mapRangeToOriginal(fragment, convertRange(fragment, edit.span))
);

return TextEdit.replace(
range,
this.fixIndentationOfImports(edit.newText, range, document)
return this.fixIndentationOfImports(
TextEdit.replace(range, edit.newText),
document
);
})
);
Expand All @@ -162,11 +162,12 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
];
}

private fixIndentationOfImports(edit: string, range: Range, document: Document): string {
// "Organize Imports" will have edits that delete all imports by return empty edits
// and one edit which contains all the organized imports. Fix indentation
private fixIndentationOfImports(edit: TextEdit, document: Document): TextEdit {
// "Organize Imports" will have edits that delete a group of imports by return empty edits
// and one edit which contains all the organized imports of the group. Fix indentation
// of that one by prepending all lines with the indentation of the first line.
if (!edit || range.start.character === 0) {
const { newText, range } = edit;
if (!newText || range.start.character === 0) {
return edit;
}

Expand All @@ -175,7 +176,21 @@ export class CodeActionsProviderImpl implements CodeActionsProvider {
if (leadingChars.trim() !== '') {
return edit;
}
return modifyLines(edit, (line, idx) => (idx === 0 || !line ? line : leadingChars + line));

const fixedNewText = modifyLines(edit.newText, (line, idx) =>
idx === 0 || !line ? line : leadingChars + line
);

if (range.end.character > 0) {
const endLine = getLineAtPosition(range.start, document.getText());
const isIndent = !endLine.substring(0, range.start.character).trim();

if (isIndent && endLine.trim()) {
range.end.character = 0;
}
}

return TextEdit.replace(range, fixedNewText);
}

private checkRemoveImportCodeActionRange(
Expand Down
Expand Up @@ -25,7 +25,7 @@ import {
import { LSConfigManager } from '../../../ls-config';
import { flatten, getRegExpMatches, isNotNullOrUndefined, pathToUrl } from '../../../utils';
import { AppCompletionItem, AppCompletionList, CompletionsProvider } from '../../interfaces';
import { ComponentPartInfo } from '../ComponentInfoProvider';
import { ComponentInfoProvider, ComponentPartInfo } from '../ComponentInfoProvider';
import { SvelteDocumentSnapshot, SvelteSnapshotFragment } from '../DocumentSnapshot';
import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
import { getMarkdownDocumentation } from '../previewer';
Expand All @@ -37,9 +37,9 @@ import {
scriptElementKindToCompletionItemKind
} from '../utils';
import { getJsDocTemplateCompletion } from './getJsDocTemplateCompletion';
import { getComponentAtPosition, isPartOfImportStatement } from './utils';
import { findContainingNode, getComponentAtPosition, isPartOfImportStatement } from './utils';

export interface CompletionEntryWithIdentifer extends ts.CompletionEntry, TextDocumentIdentifier {
export interface CompletionEntryWithIdentifier extends ts.CompletionEntry, TextDocumentIdentifier {
position: Position;
}

Expand All @@ -48,10 +48,10 @@ type validTriggerCharacter = '.' | '"' | "'" | '`' | '/' | '@' | '<' | '#';
type LastCompletion = {
key: string;
position: Position;
completionList: AppCompletionList<CompletionEntryWithIdentifer> | null;
completionList: AppCompletionList<CompletionEntryWithIdentifier> | null;
};

export class CompletionsProviderImpl implements CompletionsProvider<CompletionEntryWithIdentifer> {
export class CompletionsProviderImpl implements CompletionsProvider<CompletionEntryWithIdentifier> {
constructor(
private readonly lsAndTsDocResolver: LSAndTSDocResolver,
private readonly configManager: LSConfigManager
Expand Down Expand Up @@ -79,7 +79,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
position: Position,
completionContext?: CompletionContext,
cancellationToken?: CancellationToken
): Promise<AppCompletionList<CompletionEntryWithIdentifer> | null> {
): Promise<AppCompletionList<CompletionEntryWithIdentifier> | null> {
if (isInTag(position, document.styleInfo)) {
return null;
}
Expand Down Expand Up @@ -163,11 +163,10 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
right: /[^\w$:]/
});

const componentInfo = getComponentAtPosition(lang, document, tsDoc, position);
const eventAndSlotLetCompletions = this.getEventAndSlotLetCompletions(
lang,
componentInfo,
document,
tsDoc,
position,
wordRange
);

Expand All @@ -179,12 +178,22 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
return null;
}

const completions =
let completions =
lang.getCompletionsAtPosition(filePath, offset, {
...userPreferences,
triggerCharacter: validTriggerCharacter
})?.entries || [];

if (!completions.length) {
completions =
this.jsxTransformationPropStringLiteralCompletion(
lang,
componentInfo,
offset,
tsDoc
) ?? [];
}

if (completions.length === 0 && eventAndSlotLetCompletions.length === 0) {
return tsDoc.parserError ? CompletionList.create([], true) : null;
}
Expand Down Expand Up @@ -246,14 +255,11 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
}

private getEventAndSlotLetCompletions(
lang: ts.LanguageService,
componentInfo: ComponentInfoProvider | null,
doc: Document,
tsDoc: SvelteDocumentSnapshot,
originalPosition: Position,
wordRange: { start: number; end: number }
): Array<AppCompletionItem<CompletionEntryWithIdentifer>> {
const componentInfo = getComponentAtPosition(lang, doc, tsDoc, originalPosition);
if (!componentInfo) {
): Array<AppCompletionItem<CompletionEntryWithIdentifier>> {
if (componentInfo === null) {
return [];
}

Expand Down Expand Up @@ -285,7 +291,7 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn
uri: string,
position: Position,
existingImports: Set<string>
): AppCompletionItem<CompletionEntryWithIdentifer> | null {
): AppCompletionItem<CompletionEntryWithIdentifier> | null {
const completionLabelAndInsert = this.getCompletionLabelAndInsert(fragment, comp);
if (!completionLabelAndInsert) {
return null;
Expand Down Expand Up @@ -435,9 +441,9 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn

async resolveCompletion(
document: Document,
completionItem: AppCompletionItem<CompletionEntryWithIdentifer>,
completionItem: AppCompletionItem<CompletionEntryWithIdentifier>,
cancellationToken?: CancellationToken
): Promise<AppCompletionItem<CompletionEntryWithIdentifer>> {
): Promise<AppCompletionItem<CompletionEntryWithIdentifier>> {
const { data: comp } = completionItem;
const { tsDoc, lang, userPreferences } = await this.lsAndTsDocResolver.getLSAndTSDoc(
document
Expand Down Expand Up @@ -635,6 +641,49 @@ export class CompletionsProviderImpl implements CompletionsProvider<CompletionEn

return importText;
}

private jsxTransformationPropStringLiteralCompletion(
lang: ts.LanguageService,
componentInfo: ComponentInfoProvider | null,
position: number,
tsDoc: SvelteDocumentSnapshot
) {
if (!componentInfo || this.configManager.getConfig().svelte.useNewTransformation) {
return null;
}

const program = lang.getProgram();
const sourceFile = program?.getSourceFile(tsDoc.filePath);
if (!sourceFile) {
return null;
}

const jsxAttribute = findContainingNode(
sourceFile,
{ start: position, length: 0 },
ts.isJsxAttribute
);
if (
!jsxAttribute ||
!jsxAttribute.initializer ||
!ts.isStringLiteral(jsxAttribute.initializer)
) {
return null;
}

const replacementSpan = jsxAttribute.initializer.getWidth()
? {
// skip quote
start: jsxAttribute.initializer.getStart() + 1,
length: jsxAttribute.initializer.getWidth() - 2
}
: undefined;

return componentInfo.getProps(jsxAttribute.name.getText()).map((item) => ({
...item,
replacementSpan
}));
}
}

const beginOfDocumentRange = Range.create(Position.create(0, 0), Position.create(0, 0));
Expand Down
@@ -1,7 +1,7 @@
import ts from 'typescript';
import { Location, Position, ReferenceContext } from 'vscode-languageserver';
import { Document } from '../../../lib/documents';
import { pathToUrl } from '../../../utils';
import { flatten, pathToUrl } from '../../../utils';
import { FindReferencesProvider } from '../../interfaces';
import { LSAndTSDocResolver } from '../LSAndTSDocResolver';
import { convertToLocationRange, hasNonZeroRange } from '../utils';
Expand All @@ -18,7 +18,7 @@ export class FindReferencesProviderImpl implements FindReferencesProvider {
const { lang, tsDoc } = await this.getLSAndTSDoc(document);
const fragment = tsDoc.getFragment();

const references = lang.getReferencesAtPosition(
Copy link
Member Author

Choose a reason for hiding this comment

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

Don't know what's the difference between these two methods. But the one we used before no longer returns isDefinition

Copy link
Member

Choose a reason for hiding this comment

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

From a quick TS codebase search it seems that findReferences is like getReferencesAtPosition with an additional step afterwards, at least they both call getReferencedSymbolsForNode which does the actual work. Not sure what exactly changed that the definition is no longer part of it. Maybe they filter out the definition-position already now, which means we can keep using and just remove the check? Edit: When rewriting the store transformation and do additional logic in references/rename/etc instead, we need the definition to appear, so it would be bad if the definition is filtered out by TS now, and using findReferences would be better.

Copy link
Member Author

Choose a reason for hiding this comment

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

Found a place where it does make a difference. If you use a go-to reference on the definition.
It would invoke includeDeclaration: true. Previously if there is only one reference it would take you to the reference. if we use the findReferences and remove the check it would not take you to the reference but show both reference and definition.

const references = lang.findReferences(
tsDoc.filePath,
fragment.offsetAt(fragment.getGeneratedPosition(position))
);
Expand All @@ -30,7 +30,7 @@ export class FindReferencesProviderImpl implements FindReferencesProvider {
docs.set(tsDoc.filePath, { fragment, snapshot: tsDoc });

const locations = await Promise.all(
references
flatten(references.map((ref) => ref.references))
.filter((ref) => context.includeDeclaration || !ref.isDefinition)
.filter(notInGeneratedCode(tsDoc.getFullText()))
.map(async (ref) => {
Expand Down