Skip to content

Commit

Permalink
chore: Bindable and Svelte 5 interop (#2336)
Browse files Browse the repository at this point in the history 
- $bindable() support, adjust language server to accomodate for weird error message types and binding shorthand rename
- run Svelte 5 in CI
- remove programmatic implicit children handling, it's handled within Svelte's component types by now
- cannot bind to exports in runes mode
  • Loading branch information
@dummdidumm
dummdidumm committed Apr 25, 2024
1 parent 4c8c0db commit 957b8d6
Show file tree
Hide file tree
Showing 94 changed files with 1,589 additions and 311 deletions.
25 changes: 25 additions & 0 deletions .github/workflows/CI.yml
View file
Expand Up @@ -23,6 +23,31 @@ jobs:
env:
CI: true

test-svelte5:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- uses: pnpm/action-setup@v2.2.4
- uses: actions/setup-node@v3
with:
node-version: "18.x"
cache: pnpm

# Lets us use one-liner JSON manipulations the package.json files
- run: "npm install -g json"

# Get projects set up
- run: json -I -f package.json -e 'this.pnpm={"overrides":{"svelte":"^5.0.0-next.100"}}'
- run: pnpm install --no-frozen-lockfile
- run: pnpm bootstrap
- run: pnpm build

# Run any tests
- run: pnpm test
env:
CI: true

lint:
runs-on: ubuntu-latest

Expand Down
2 changes: 1 addition & 1 deletion packages/language-server/src/plugins/svelte/features/getDiagnostics.ts
View file
Expand Up @@ -63,7 +63,7 @@ async function tryGetDiagnostics(
if (cancellationToken?.isCancellationRequested) {
return [];
}
return (((res.stats as any)?.warnings || res.warnings || []) as Warning[])
return (res.warnings || [])
.filter((warning) => settings[warning.code] !== 'ignore')
.map((warning) => {
const start = warning.start || { line: 1, column: 0 };
Expand Down
6 changes: 6 additions & 0 deletions packages/language-server/src/plugins/typescript/DocumentSnapshot.ts
View file
Expand Up @@ -87,6 +87,7 @@ export namespace DocumentSnapshot {
document,
parserError,
scriptKind,
options.version,
text,
nrPrependedLines,
exportedNames,
Expand Down Expand Up @@ -272,13 +273,18 @@ export class SvelteDocumentSnapshot implements DocumentSnapshot {
public readonly parent: Document,
public readonly parserError: ParserError | null,
public readonly scriptKind: ts.ScriptKind,
public readonly svelteVersion: string | undefined,
private readonly text: string,
private readonly nrPrependedLines: number,
private readonly exportedNames: IExportedNames,
private readonly tsxMap?: EncodedSourceMap,
private readonly htmlAst?: TemplateNode
) {}

get isSvelte5Plus() {
return Number(this.svelteVersion?.split('.')[0]) >= 5;
}

get filePath() {
return this.parent.getFilePath() || '';
}
Expand Down
61 changes: 52 additions & 9 deletions packages/language-server/src/plugins/typescript/features/DiagnosticsProvider.ts
View file
Expand Up @@ -21,7 +21,14 @@ import {
isStoreVariableIn$storeDeclaration,
get$storeOffsetOf$storeDeclaration
} from './utils';
import { not, flatten, passMap, swapRangeStartEndIfNecessary, memoize } from '../../../utils';
import {
not,
flatten,
passMap,
swapRangeStartEndIfNecessary,
memoize,
traverseTypeString
} from '../../../utils';
import { LSConfigManager } from '../../../ls-config';
import { isAttributeName, isEventHandler } from '../svelte-ast-utils';

Expand All @@ -37,7 +44,10 @@ export enum DiagnosticCode {
DUPLICATED_JSX_ATTRIBUTES = 17001, // "JSX elements cannot have multiple attributes with the same name."
DUPLICATE_IDENTIFIER = 2300, // "Duplicate identifier 'xxx'"
MULTIPLE_PROPS_SAME_NAME = 1117, // "An object literal cannot have multiple properties with the same name in strict mode."
TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y = 2345, // "Argument of type '..' is not assignable to parameter of type '..'."
ARG_TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y = 2345, // "Argument of type '..' is not assignable to parameter of type '..'."
TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y = 2322, // "Type '..' is not assignable to type '..'."
TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y_DID_YOU_MEAN = 2820, // "Type '..' is not assignable to type '..'. Did you mean '...'?"
UNKNOWN_PROP = 2353, // "Object literal may only specify known properties, and '...' does not exist in type '...'"
MISSING_PROPS = 2739, // "Type '...' is missing the following properties from type '..': ..."
MISSING_PROP = 2741, // "Property '..' is missing in type '..' but required in type '..'."
NO_OVERLOAD_MATCHES_CALL = 2769, // "No overload matches this call"
Expand Down Expand Up @@ -101,7 +111,7 @@ export class DiagnosticsProviderImpl implements DiagnosticsProvider {
for (const diagnostic of diagnostics) {
if (
(diagnostic.code === DiagnosticCode.NO_OVERLOAD_MATCHES_CALL ||
diagnostic.code === DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y) &&
diagnostic.code === DiagnosticCode.ARG_TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y) &&
!notGenerated(diagnostic)
) {
if (isStoreVariableIn$storeDeclaration(tsDoc.getFullText(), diagnostic.start!)) {
Expand Down Expand Up @@ -147,7 +157,7 @@ export class DiagnosticsProviderImpl implements DiagnosticsProvider {
.map(mapRange(tsDoc, document, lang))
.filter(hasNoNegativeLines)
.filter(isNoFalsePositive(document, tsDoc))
.map(enhanceIfNecessary)
.map(adjustIfNecessary)
.map(swapDiagRangeStartEndIfNecessary);
}

Expand Down Expand Up @@ -180,9 +190,11 @@ function mapRange(
}

if (
[DiagnosticCode.MISSING_PROP, DiagnosticCode.MISSING_PROPS].includes(
([DiagnosticCode.MISSING_PROP, DiagnosticCode.MISSING_PROPS].includes(
diagnostic.code as number
) &&
) ||
(DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y &&
diagnostic.message.includes("'PropsWithChildren<"))) &&
!hasNonZeroRange({ range })
) {
const node = getNodeIfIsInStartTag(document.html, document.offsetAt(range.start));
Expand Down Expand Up @@ -286,11 +298,11 @@ function isNoUsedBeforeAssigned(
}

/**
* Some diagnostics have JSX-specific nomenclature. Enhance them for more clarity.
* Some diagnostics have JSX-specific or confusing nomenclature. Enhance/adjust them for more clarity.
*/
function enhanceIfNecessary(diagnostic: Diagnostic): Diagnostic {
function adjustIfNecessary(diagnostic: Diagnostic): Diagnostic {
if (
diagnostic.code === DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y &&
diagnostic.code === DiagnosticCode.ARG_TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y &&
diagnostic.message.includes('ConstructorOfATypedSvelteComponent')
) {
return {
Expand All @@ -315,6 +327,37 @@ function enhanceIfNecessary(diagnostic: Diagnostic): Diagnostic {
};
}

if (
(diagnostic.code === DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y ||
diagnostic.code === DiagnosticCode.TYPE_X_NOT_ASSIGNABLE_TO_TYPE_Y_DID_YOU_MEAN) &&
diagnostic.message.includes("'Bindable<")
) {
const countBindable = (diagnostic.message.match(/'Bindable\</g) || []).length;
const countBinding = (diagnostic.message.match(/'Binding\</g) || []).length;
if (countBindable === 1 && countBinding === 0) {
// Remove distracting Bindable<...> from diagnostic message
const start = diagnostic.message.indexOf("'Bindable<");
const startType = start + "'Bindable".length;
const end = traverseTypeString(diagnostic.message, startType, '>');
diagnostic.message =
diagnostic.message.substring(0, start + 1) +
diagnostic.message.substring(startType + 1, end) +
diagnostic.message.substring(end + 1);
} else if (countBinding === 3 && countBindable === 1) {
// Only keep Type '...' is not assignable to type '...' in
// Type Bindings<...> is not assignable to type Bindable<...>, Type Binding<...> is not assignable to type Bindable<...>, Type '...' is not assignable to type '...'
const lines = diagnostic.message.split('\n');
if (lines.length === 3) {
diagnostic.message = lines[2].trimStart();
}
}

return {
...diagnostic,
message: diagnostic.message
};
}

return diagnostic;
}

Expand Down
108 changes: 83 additions & 25 deletions packages/language-server/src/plugins/typescript/features/RenameProvider.ts
View file
Expand Up @@ -37,6 +37,9 @@ interface TsRenameLocation extends ts.RenameLocation {
newName?: string;
}

const bind = 'bind:';
const bindShortHandGeneratedLength = ':__sveltets_2_binding('.length;

export class RenameProviderImpl implements RenameProvider {
constructor(
private readonly lsAndTsDocResolver: LSAndTSDocResolver,
Expand Down Expand Up @@ -73,7 +76,7 @@ export class RenameProviderImpl implements RenameProvider {

const renameLocations = lang.findRenameLocations(
tsDoc.filePath,
offset,
offset + (renameInfo.bindShorthand || 0),
false,
false,
true
Expand Down Expand Up @@ -157,6 +160,7 @@ export class RenameProviderImpl implements RenameProvider {
):
| (ts.RenameInfoSuccess & {
isStore?: boolean;
bindShorthand?: number;
})
| null {
// Don't allow renames in error-state, because then there is no generated svelte2tsx-code
Expand All @@ -165,6 +169,15 @@ export class RenameProviderImpl implements RenameProvider {
return null;
}

const svelteNode = tsDoc.svelteNodeAt(originalPosition);

let bindOffset = 0;
const bindingShorthand = this.getBindingShorthand(tsDoc, originalPosition, svelteNode);
if (bindingShorthand) {
bindOffset = bindingShorthand.end - bindingShorthand.start;
generatedOffset += bindShortHandGeneratedLength + bindOffset;
}

const renameInfo = lang.getRenameInfo(tsDoc.filePath, generatedOffset, {
allowRenameOfImportPath: false
});
Expand All @@ -178,7 +191,6 @@ export class RenameProviderImpl implements RenameProvider {
return null;
}

const svelteNode = tsDoc.svelteNodeAt(originalPosition);
if (
isInHTMLTagRange(doc.html, doc.offsetAt(originalPosition)) ||
isAttributeName(svelteNode, 'Element') ||
Expand All @@ -188,16 +200,22 @@ export class RenameProviderImpl implements RenameProvider {
}

// If $store is renamed, only allow rename for $|store|
if (tsDoc.getFullText().charAt(renameInfo.triggerSpan.start) === '$') {
const text = tsDoc.getFullText();
if (text.charAt(renameInfo.triggerSpan.start) === '$') {
const definition = lang.getDefinitionAndBoundSpan(tsDoc.filePath, generatedOffset)
?.definitions?.[0];
if (definition && isTextSpanInGeneratedCode(tsDoc.getFullText(), definition.textSpan)) {
if (definition && isTextSpanInGeneratedCode(text, definition.textSpan)) {
renameInfo.triggerSpan.start++;
renameInfo.triggerSpan.length--;
(renameInfo as any).isStore = true;
}
}

if (bindOffset) {
renameInfo.triggerSpan.start -= bindShortHandGeneratedLength + bindOffset;
(renameInfo as any).bindShorthand = bindShortHandGeneratedLength + bindOffset;
}

return renameInfo;
}

Expand Down Expand Up @@ -325,7 +343,6 @@ export class RenameProviderImpl implements RenameProvider {
replacementsForProp,
snapshots
);
const bind = 'bind:';

// Adjust shorthands
return renameLocations.map((location) => {
Expand All @@ -351,31 +368,29 @@ export class RenameProviderImpl implements RenameProvider {

const { parent } = snapshot;

let rangeStart = parent.offsetAt(location.range.start);
let suffixText = location.suffixText?.trimStart();

// suffix is of the form `: oldVarName` -> hints at a shorthand
if (!suffixText?.startsWith(':') || !getNodeIfIsInStartTag(parent.html, rangeStart)) {
return location;
}

const original = parent.getText({
start: Position.create(
location.range.start.line,
location.range.start.character - bind.length
),
end: location.range.end
});

if (original.startsWith(bind)) {
const bindingShorthand = this.getBindingShorthand(snapshot, location.range.start);
if (bindingShorthand) {
// bind:|foo| -> bind:|newName|={foo}
const name = parent
.getText()
.substring(bindingShorthand.start, bindingShorthand.end);
return {
...location,
prefixText: '',
suffixText: `={${original.slice(bind.length)}}`
suffixText: `={${name}}`
};
}

let rangeStart = parent.offsetAt(location.range.start);

// suffix is of the form `: oldVarName` -> hints at a shorthand
if (
!location.suffixText?.trimStart()?.startsWith(':') ||
!getNodeIfIsInStartTag(parent.html, rangeStart)
) {
return location;
}

if (snapshot.getOriginalText().charAt(rangeStart - 1) === '{') {
// {|foo|} -> |{foo|}
rangeStart--;
Expand Down Expand Up @@ -582,8 +597,6 @@ export class RenameProviderImpl implements RenameProvider {
renameLocations: TsRenameLocation[],
snapshots: SnapshotMap
): TsRenameLocation[] {
const bind = 'bind:';

return renameLocations.map((location) => {
const sourceFile = lang.getProgram()?.getSourceFile(location.fileName);

Expand Down Expand Up @@ -625,6 +638,38 @@ export class RenameProviderImpl implements RenameProvider {
suffixText: '}'
};
}

if (snapshot.isSvelte5Plus) {
const bindingShorthand = this.getBindingShorthand(
snapshot,
location.range.start
);
if (bindingShorthand) {
const name = parent
.getText()
.substring(bindingShorthand.start, bindingShorthand.end);
const start = {
line: location.range.start.line,
character: location.range.start.character - name.length
};
// If binding is followed by the closing tag, start is one character too soon,
// else binding is ending one character too far
if (parent.getText().charAt(parent.offsetAt(start)) === ':') {
start.character++;
} else {
location.range.end.character--;
}
return {
...location,
range: {
start: start,
end: location.range.end
},
prefixText: name + '={',
suffixText: '}'
};
}
}
}

if (!prefixText || prefixText.slice(-1) !== ':') {
Expand Down Expand Up @@ -668,4 +713,17 @@ export class RenameProviderImpl implements RenameProvider {
return location;
});
}

private getBindingShorthand(
snapshot: SvelteDocumentSnapshot,
position: Position,
svelteNode = snapshot.svelteNodeAt(position)
) {
if (
svelteNode?.parent?.type === 'Binding' &&
svelteNode.parent.expression.end === svelteNode.parent.end
) {
return svelteNode.parent.expression;
}
}
}
1 change: 1 addition & 0 deletions packages/language-server/src/plugins/typescript/svelte-ast-utils.ts
View file
Expand Up @@ -8,6 +8,7 @@ export interface SvelteNode {
end: number;
type: string;
parent?: SvelteNode;
[key: string]: any;
}

type HTMLLike = 'Element' | 'InlineComponent' | 'Body' | 'Window';
Expand Down

0 comments on commit 957b8d6

Please sign in to comment.