Skip to content

Commit

Permalink
fix no-missing-import getting confused on incremental file changes (#338
Browse files Browse the repository at this point in the history
)
  • Loading branch information
AndrewJakubowicz authored Jan 5, 2024
1 parent 6408719 commit 9f44f6f
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,6 @@ export class DefaultLitAnalyzerContext implements LitAnalyzerContext {

// Build a graph of component dependencies
const res = parseDependencies(file, this);
this.dependencyStore.importedComponentDefinitionsInFile.set(file.fileName, res);
this.dependencyStore.absorbComponentDefinitionsForFile(file, res);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,16 @@ function emitDirectModuleImportWithName(moduleSpecifier: string, node: Node, con
if (cache != null) {
result = context.ts.resolveModuleNameFromCache(moduleSpecifier, node.getSourceFile().fileName, cache, mode);
}
if (result == null) {
// Result could not be found from the cache, try and resolve module without using the
// cache.
result = context.ts.resolveModuleName(
moduleSpecifier,
node.getSourceFile().fileName,
context.program.getCompilerOptions(),
context.ts.createCompilerHost(context.program.getCompilerOptions())
);
}
}

if (result?.resolvedModule?.resolvedFileName != null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,3 @@
export interface AnalyzerDependencyStore {
hasTagNameBeenImported(fileName: string, tagName: string): boolean;
}

//importedComponentDefinitionsInFile = new Map<string, ComponentDefinition[]>();

/**
* Returns if a component for a specific file has been imported.
* @param fileName
* @param tagName
*/
/*hasTagNameBeenImported(fileName: string, tagName: string): boolean {
for (const file of this.importedComponentDefinitionsInFile.get(fileName) || []) {
if (file.tagName === tagName) {
return true;
}
}
return false;
}*/
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ComponentDefinition } from "web-component-analyzer";
import { AnalyzerDependencyStore } from "../analyzer-dependency-store.js";

export class DefaultAnalyzerDependencyStore implements AnalyzerDependencyStore {
importedComponentDefinitionsInFile = new Map<string, ComponentDefinition[]>();
private importedComponentDefinitionsInFile = new Map<string, ComponentDefinition[]>();

absorbComponentDefinitionsForFile(sourceFile: SourceFile, result: ComponentDefinition[]): void {
this.importedComponentDefinitionsInFile.set(sourceFile.fileName, result);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import "./my-defined-element.js";

// Pretending this is the Lit html function
// eslint-disable-next-line @typescript-eslint/no-explicit-any
declare const html: any;

html`<my-defined-element></my-defined-element><my-other-element></my-other-element>`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export class MyDefinedElement extends HTMLElement {}

customElements.define("my-defined-element", MyDefinedElement);

declare global {
interface HTMLElementTagNameMap {
"my-defined-element": MyDefinedElement;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export class MyOtherElement extends HTMLElement {}

customElements.define("my-other-element", MyOtherElement);

declare global {
interface HTMLElementTagNameMap {
"my-other-element": MyOtherElement;
}
}
58 changes: 43 additions & 15 deletions packages/vscode-lit-plugin/src/test/simple-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,20 @@ import * as path from "path";
import * as vscode from "vscode";
// import * as litPlugin from "../extension.js";

// wait until the TS language server is ready and diagnostics are produced
async function getDiagnostics(docUri: vscode.Uri, retries = 1000) {
for (let i = 0; i < retries; i++) {
const diagnostics = vscode.languages.getDiagnostics(docUri);
if (diagnostics.length > 0) {
return diagnostics;
}
// Is there a better way to wait for the ts server to be ready?
// Maybe we can listen for the event that displays and hides the "initializing TS/JS language features" message?
await new Promise(resolve => setTimeout(resolve, 100));
}
throw new Error("No diagnostics found");
}

suite("Extension Test Suite", () => {
after(() => {
vscode.window.showInformationMessage("All tests done!");
Expand All @@ -25,27 +39,41 @@ suite("Extension Test Suite", () => {
const doc = await vscode.workspace.openTextDocument(vscode.Uri.file(path.join(__dirname, "../../src/test/fixtures/missing-elem-type.ts")));
await vscode.window.showTextDocument(doc);

// wait until the TS language server is ready and diagnostics are produced
async function getDiagnostics() {
for (let i = 0; i < 1000; i++) {
const diagnostics = vscode.languages.getDiagnostics(doc.uri);
if (diagnostics.length > 0) {
return diagnostics;
}
// Is there a better way to wait for the ts server to be ready?
// Maybe we can listen for the event that displays and hides the "initializing TS/JS language features" message?
await new Promise(resolve => setTimeout(resolve, 100));
}
throw new Error("No diagnostics found");
}

const diagnostics = await getDiagnostics();
const diagnostics = await getDiagnostics(doc.uri);
assert.deepStrictEqual(
diagnostics.map(d => d.message),
["'my-element' has not been registered on HTMLElementTagNameMap"]
);
});

test("We detect no-missing-import properly", async () => {
const config = vscode.workspace.getConfiguration();
config.update("lit-plugin.logging", "verbose", true);
config.update("lit-plugin.rules.no-missing-import", "error", true);
const doc = await vscode.workspace.openTextDocument(vscode.Uri.file(path.join(__dirname, "../../src/test/fixtures/missing-import.ts")));
const editor = await vscode.window.showTextDocument(doc);

const diagnostics = await getDiagnostics(doc.uri);
assert.deepStrictEqual(
diagnostics.map(d => d.message),
["Missing import for <my-other-element>\n You can disable this check by disabling the 'no-missing-import' rule."]
);

// now add the fix
const editRange = doc.lineAt(0).range.start;
editor.edit(builder => {
if (!editor) {
throw new Error("No editor found");
}
editor.insertSnippet(new vscode.SnippetString("import './my-other-element';\n"), editRange);
});

// give it some time to settle
await new Promise(resolve => setTimeout(resolve, 1000));

assert.rejects(getDiagnostics(doc.uri, 3), "Expected rejection as no diagnostics will be found.");
});

test("We generate completions", async () => {
const doc = await vscode.workspace.openTextDocument(vscode.Uri.file(path.join(__dirname, "../../src/test/fixtures/completions.ts")));
const editor = await vscode.window.showTextDocument(doc);
Expand Down

0 comments on commit 9f44f6f

Please sign in to comment.