Skip to content

Commit

Permalink
fix(language-service): resolve to the pre-compiled style when compile…
Browse files Browse the repository at this point in the history
…d css url is provided

With this commit, the language service will first try to locate a
pre-compiled style file with the same name when a `css` is provided in
the `styleUrls`. This prevents a missing resource diagnostic for when the
compiled file is not available in the language service environment and also
allows "go to definition" to go to that pre-compiled file.

Fixes angular/vscode-ng-language-service#1263
  • Loading branch information
atscott committed Apr 9, 2021
1 parent 10a7c87 commit 5d1d74a
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 9 deletions.
14 changes: 9 additions & 5 deletions packages/compiler-cli/src/ngtsc/annotations/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as ts from 'typescript';

import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../cycles';
import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics';
import {ComponentResourceNotFoundContext} from '../../diagnostics/src/error';
import {absoluteFrom, relative} from '../../file_system';
import {DefaultImportRecorder, ImportedFile, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
import {DependencyTracker} from '../../incremental/api';
Expand Down Expand Up @@ -322,7 +323,6 @@ export class ComponentDecoratorHandler implements
this.literalCache.delete(decorator);

let diagnostics: ts.Diagnostic[]|undefined;
let isPoisoned = false;
// @Component inherits @Directive, so begin by extracting the @Directive metadata and building
// on it.
const directiveResult = extractDirectiveMetadata(
Expand Down Expand Up @@ -420,9 +420,13 @@ export class ComponentDecoratorHandler implements
styleUrl.source === ResourceTypeForDiagnostics.StylesheetFromDecorator ?
ResourceTypeForDiagnostics.StylesheetFromDecorator :
ResourceTypeForDiagnostics.StylesheetFromTemplate;
diagnostics.push(
this.makeResourceNotFoundError(styleUrl.url, styleUrl.nodeForError, resourceType)
.toDiagnostic());
const diagnostic: ts.DiagnosticWithLocation&ComponentResourceNotFoundContext = {
...this.makeResourceNotFoundError(styleUrl.url, styleUrl.nodeForError, resourceType)
.toDiagnostic(),
fromFile: containingFile,
url: styleUrl.url,
};
diagnostics.push(diagnostic);
}
}

Expand Down Expand Up @@ -501,7 +505,7 @@ export class ComponentDecoratorHandler implements
styles: styleResources,
template: templateResource,
},
isPoisoned,
isPoisoned: false,
},
diagnostics,
};
Expand Down
7 changes: 6 additions & 1 deletion packages/compiler-cli/src/ngtsc/core/api/src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,13 @@ export interface ResourceHost {
/**
* Converts a file path for a resource that is used in a source file or another resource
* into a filepath.
*
* The optional `fallbackResolve` method can be used as a way to attempt a fallback resolution if
* the implementation's `resourceNameToFileName` resolution fails.
*/
resourceNameToFileName(resourceName: string, containingFilePath: string): string|null;
resourceNameToFileName(
resourceName: string, containingFilePath: string,
fallbackResolve?: (url: string, fromFile: string) => string | null): string|null;

/**
* Load a referenced resource either statically or asynchronously. If the host returns a
Expand Down
10 changes: 10 additions & 0 deletions packages/compiler-cli/src/ngtsc/diagnostics/src/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,13 @@ export function makeRelatedInformation(
export function isFatalDiagnosticError(err: any): err is FatalDiagnosticError {
return err._isFatalDiagnosticError === true;
}

export interface ComponentResourceNotFoundContext {
fromFile: string;
url: string;
}

export function isResourceNotFoundWithContextError(err: ts.Diagnostic):
err is ts.DiagnosticWithLocation&ComponentResourceNotFoundContext {
return err.code === ngErrorCode(ErrorCode.COMPONENT_RESOURCE_NOT_FOUND) && 'fromFile' in err;
}
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/resource/src/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ export class AdapterResourceLoader implements ResourceLoader {
resolve(url: string, fromFile: string): string {
let resolvedUrl: string|null = null;
if (this.adapter.resourceNameToFileName) {
resolvedUrl = this.adapter.resourceNameToFileName(url, fromFile);
resolvedUrl = this.adapter.resourceNameToFileName(
url, fromFile, (url: string, fromFile: string) => this.fallbackResolve(url, fromFile));
} else {
resolvedUrl = this.fallbackResolve(url, fromFile);
}
Expand Down
18 changes: 18 additions & 0 deletions packages/language-service/ivy/adapters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,24 @@ export class LanguageServiceAdapter implements NgCompilerAdapter {
this.rootDirs = getRootDirs(this, project.getCompilationSettings());
}

resourceNameToFileName(
url: string, fromFile: string,
fallbackResolve?: (url: string, fromFile: string) => string | null): string|null {
// If we are trying to resolve a `.css` file, see if we can find a pre-compiled file with the
// same name instead. That way, we can provide go-to-definition for the pre-compiled files which
// would generally be the desired behavior.
if (url.endsWith('.css')) {
const styleUrl = p.resolve(fromFile, '..', url);
for (const ext of ['.scss', '.sass', '.less', '.styl']) {
const precompiledFileUrl = styleUrl.replace(/(\.css)$/, ext);
if (this.fileExists(precompiledFileUrl)) {
return precompiledFileUrl;
}
}
}
return fallbackResolve?.(url, fromFile) ?? null;
}

isShim(sf: ts.SourceFile): boolean {
return isShim(sf);
}
Expand Down
2 changes: 0 additions & 2 deletions packages/language-service/ivy/compiler_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,10 @@
import {CompilationTicket, freshCompilationTicket, incrementalFromCompilerTicket, NgCompiler, resourceChangeTicket} from '@angular/compiler-cli/src/ngtsc/core';
import {NgCompilerOptions} from '@angular/compiler-cli/src/ngtsc/core/api';
import {TrackedIncrementalBuildStrategy} from '@angular/compiler-cli/src/ngtsc/incremental';
import {ActivePerfRecorder} from '@angular/compiler-cli/src/ngtsc/perf';
import {TypeCheckingProgramStrategy} from '@angular/compiler-cli/src/ngtsc/typecheck/api';
import * as ts from 'typescript/lib/tsserverlibrary';

import {LanguageServiceAdapter} from './adapters';
import {isExternalTemplate} from './utils';

/**
* Manages the `NgCompiler` instance which backs the language service, updating or replacing it as
Expand Down
25 changes: 25 additions & 0 deletions packages/language-service/ivy/test/definitions_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,31 @@ describe('definitions', () => {
assertFileNames([def, def2], ['dir2.ts', 'dir.ts']);
});

it('should go to the pre-compiled style sheet', () => {
initMockFileSystem('Native');
const files = {
'app.ts': `
import {Component} from '@angular/core';
@Component({
template: '',
styleUrls: ['./style.css'],
})
export class AppCmp {}
`,
'style.scss': '',
};
const env = LanguageServiceTestEnv.setup();
const project = createModuleAndProjectWithDeclarations(env, 'test', files);
const appFile = project.openFile('app.ts');
appFile.moveCursorToText(`['./styl¦e.css']`);
const {textSpan, definitions} = getDefinitionsAndAssertBoundSpan(env, appFile);
expect(appFile.contents.substr(textSpan.start, textSpan.length)).toEqual('./style.css');

expect(definitions.length).toEqual(1);
assertFileNames(definitions, ['style.scss']);
});

function getDefinitionsAndAssertBoundSpan(env: LanguageServiceTestEnv, file: OpenBuffer) {
env.expectNoSourceDiagnostics();
const definitionAndBoundSpan = file.getDefinitionAndBoundSpan();
Expand Down
26 changes: 26 additions & 0 deletions packages/language-service/ivy/test/diagnostic_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,32 @@ describe('getSemanticDiagnostics', () => {
.toHaveBeenCalledWith(jasmine.stringMatching(
/LanguageService\#LsDiagnostics\:.*\"LsDiagnostics\":\s*\d+.*/g));
});

it('ignores style url resource missing', () => {
const files = {
'app.ts': `
import {Component} from '@angular/core';
@Component({
template: '',
styleUrls: ['./one.css', './two/two.css', './three.css', '../test/four.css', './missing.css'],
})
export class MyComponent {}
`,
'one.scss': '',
'two/two.sass': '',
'three.less': '',
'four.styl': '',
};

const project = createModuleAndProjectWithDeclarations(env, 'test', files);
const diags = project.getDiagnosticsForFile('app.ts');
expect(diags.length).toBe(1);
const diag = diags[0];
expect(diag.code).toBe(ngErrorCode(ErrorCode.COMPONENT_RESOURCE_NOT_FOUND));
expect(diag.category).toBe(ts.DiagnosticCategory.Error);
expect(getTextOfDiagnostic(diag)).toBe(`'./missing.css'`);
});
});

function getTextOfDiagnostic(diag: ts.Diagnostic): string {
Expand Down

0 comments on commit 5d1d74a

Please sign in to comment.