Skip to content

Commit

Permalink
fix(compiler-cli): catch fatal diagnostic when getting diagnostics fo…
Browse files Browse the repository at this point in the history
…r components (#50046)

This commit adds similar handling to what was done in ed817e3.
The language service calls the `getDiagnosticsForComponent` function
when the file is not a typescript file.

fixes angular/vscode-ng-language-service#1881

PR Close #50046
  • Loading branch information
atscott authored and pkozlowski-opensource committed Apr 28, 2023
1 parent c650b40 commit ce00738
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 12 deletions.
15 changes: 11 additions & 4 deletions packages/compiler-cli/src/ngtsc/core/src/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -448,11 +448,18 @@ export class NgCompiler {
const compilation = this.ensureAnalyzed();
const ttc = compilation.templateTypeChecker;
const diagnostics: ts.Diagnostic[] = [];
diagnostics.push(...ttc.getDiagnosticsForComponent(component));
try {
diagnostics.push(...ttc.getDiagnosticsForComponent(component));

const extendedTemplateChecker = compilation.extendedTemplateChecker;
if (this.options.strictTemplates && extendedTemplateChecker) {
diagnostics.push(...extendedTemplateChecker.getDiagnosticsForComponent(component));
const extendedTemplateChecker = compilation.extendedTemplateChecker;
if (this.options.strictTemplates && extendedTemplateChecker) {
diagnostics.push(...extendedTemplateChecker.getDiagnosticsForComponent(component));
}
} catch (err: unknown) {
if (!(err instanceof FatalDiagnosticError)) {
throw err;
}
diagnostics.push(err.toDiagnostic());
}
return this.addMessageTextDetails(diagnostics);
}
Expand Down
43 changes: 43 additions & 0 deletions packages/language-service/test/diagnostic_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,49 @@ describe('getSemanticDiagnostics', () => {
const diags = project.getDiagnosticsForFile('app.html');
expect(diags.length).toEqual(0);
});

it('generates diagnostic when the library does not export the host directive', () => {
const files = {
// export post module and component but not the host directive. This is not valid. We won't
// be able to import the host directive for template type checking.
'dist/post/index.d.ts': `
export { PostComponent, PostModule } from './lib/post.component';
`,
'dist/post/lib/post.component.d.ts': `
import * as i0 from "@angular/core";
export declare class HostBindDirective {
static ɵdir: i0.ɵɵDirectiveDeclaration<HostBindDirective, never, never, {}, {}, never, never, true, never>;
}
export declare class PostComponent {
static ɵcmp: i0.ɵɵComponentDeclaration<PostComponent, "lib-post", never, {}, {}, never, never, false, [{ directive: typeof HostBindDirective; inputs: {}; outputs: {}; }]>;
}
export declare class PostModule {
static ɵmod: i0.ɵɵNgModuleDeclaration<PostModule, [typeof PostComponent], never, [typeof PostComponent]>;
static ɵinj: i0.ɵɵInjectorDeclaration<PostModule>;
}
`,
'test.ts': `
import {Component} from '@angular/core';
import {PostModule} from 'post';
@Component({
templateUrl: './test.ng.html',
imports: [PostModule],
standalone: true,
})
export class Main { }
`,
'test.ng.html': '<lib-post />'
};

const tsCompilerOptions = {paths: {'post': ['dist/post']}};
const project = env.addProject('test', files, {}, tsCompilerOptions);

const diags = project.getDiagnosticsForFile('test.ng.html');
expect(diags.length).toBe(1);
expect(ts.flattenDiagnosticMessageText(diags[0].messageText, ''))
.toContain('HostBindDirective');
});
});

function getTextOfDiagnostic(diag: ts.Diagnostic): string {
Expand Down
7 changes: 5 additions & 2 deletions packages/language-service/testing/src/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,15 @@ export class LanguageServiceTestEnv {

constructor(private host: MockServerHost, private projectService: ts.server.ProjectService) {}

addProject(name: string, files: ProjectFiles, options: TestableOptions = {}): Project {
addProject(
name: string, files: ProjectFiles, angularCompilerOptions: TestableOptions = {},
tsCompilerOptions = {}): Project {
if (this.projects.has(name)) {
throw new Error(`Project ${name} is already defined`);
}

const project = Project.initialize(name, this.projectService, files, options);
const project = Project.initialize(
name, this.projectService, files, angularCompilerOptions, tsCompilerOptions);
this.projects.set(name, project);
return project;
}
Expand Down
9 changes: 5 additions & 4 deletions packages/language-service/testing/src/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export type ProjectFiles = {

function writeTsconfig(
fs: FileSystem, tsConfigPath: AbsoluteFsPath, entryFiles: AbsoluteFsPath[],
options: TestableOptions): void {
angularCompilerOptions: TestableOptions, tsCompilerOptions: {}): void {
fs.writeFile(
tsConfigPath,
JSON.stringify(
Expand All @@ -36,11 +36,12 @@ function writeTsconfig(
'dom',
'es2015',
],
...tsCompilerOptions,
},
files: entryFiles,
angularCompilerOptions: {
strictTemplates: true,
...options,
...angularCompilerOptions,
}
},
null, 2));
Expand All @@ -57,7 +58,7 @@ export class Project {

static initialize(
projectName: string, projectService: ts.server.ProjectService, files: ProjectFiles,
options: TestableOptions = {}): Project {
angularCompilerOptions: TestableOptions = {}, tsCompilerOptions = {}): Project {
const fs = getFileSystem();
const tsConfigPath = absoluteFrom(`/${projectName}/tsconfig.json`);

Expand All @@ -73,7 +74,7 @@ export class Project {
}
}

writeTsconfig(fs, tsConfigPath, entryFiles, options);
writeTsconfig(fs, tsConfigPath, entryFiles, angularCompilerOptions, tsCompilerOptions);

// Ensure the project is live in the ProjectService.
projectService.openClientFile(entryFiles[0]);
Expand Down
4 changes: 2 additions & 2 deletions packages/language-service/testing/src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function getFirstClassDeclaration(declaration: string) {

export function createModuleAndProjectWithDeclarations(
env: LanguageServiceTestEnv, projectName: string, projectFiles: ProjectFiles,
options: TestableOptions = {}, standaloneFiles: ProjectFiles = {}): Project {
angularCompilerOptions: TestableOptions = {}, standaloneFiles: ProjectFiles = {}): Project {
const externalClasses: string[] = [];
const externalImports: string[] = [];
for (const [fileName, fileContents] of Object.entries(projectFiles)) {
Expand All @@ -72,7 +72,7 @@ export function createModuleAndProjectWithDeclarations(
export class AppModule {}
`;
projectFiles['app-module.ts'] = moduleContents;
return env.addProject(projectName, {...projectFiles, ...standaloneFiles}, options);
return env.addProject(projectName, {...projectFiles, ...standaloneFiles}, angularCompilerOptions);
}

export function humanizeDocumentSpanLike<T extends ts.DocumentSpan>(
Expand Down

0 comments on commit ce00738

Please sign in to comment.