Skip to content

Commit

Permalink
fix(language-service): import the default exported component correctly (
Browse files Browse the repository at this point in the history 
#56432)

When importing a component exported by default, the `default` can't be
used as the component name.

For example:

This is the export declarations:

```ts
export default class TestComponent {}
```

Previously, the output generated by LS looked like this:

```ts
import { default } from "./test.component";
```

Now the output looks like this:

```ts
import TestComponent from "./test.component";
```

Fixes #48689

PR Close #56432
  • Loading branch information
@ivanwonder @AndrewKushnir
ivanwonder authored and AndrewKushnir committed Jun 17, 2024
1 parent d203c49 commit 67b2c33
Show file tree
Hide file tree
Showing 3 changed files with 199 additions and 22 deletions.
87 changes: 71 additions & 16 deletions packages/language-service/src/ts_utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,10 @@ function nameInExportScope(importSpecifier: ts.ImportSpecifier): string {
* `propertyName`, and if so, the name it can be referred to with in the local scope.
*/
function importHas(importDecl: ts.ImportDeclaration, propName: string): string | null {
const importClauseName = importDecl.importClause?.name?.getText();
if (propName === 'default' && importClauseName !== undefined) {
return importClauseName;
}
const bindings = importDecl.importClause?.namedBindings;
if (bindings === undefined) {
return null;
Expand Down Expand Up @@ -350,11 +354,15 @@ export function standaloneTraitOrNgModule(
/**
* Updates the imports on a TypeScript file, by ensuring the provided import is present.
* Returns the text changes, as well as the name with which the imported symbol can be referred to.
*
* When the component is exported by default, the `symbolName` is `default`, and the `declarationName`
* should be used as the import name.
*/
export function updateImportsForTypescriptFile(
tsChecker: ts.TypeChecker,
file: ts.SourceFile,
symbolName: string,
declarationName: string,
moduleSpecifier: string,
tsFileToImport: ts.SourceFile,
): [ts.TextChange[], string] {
Expand All @@ -375,19 +383,22 @@ export function updateImportsForTypescriptFile(
const existingImportDeclaration = allImports.find((decl) =>
moduleSpecifierPointsToFile(tsChecker, decl.moduleSpecifier, tsFileToImport),
);
const importName = nonCollidingImportName(allImports, symbolName);
const importName = nonCollidingImportName(
allImports,
symbolName === 'default' ? declarationName : symbolName,
);

if (existingImportDeclaration !== undefined) {
// Update an existing import declaration.
const bindings = existingImportDeclaration.importClause?.namedBindings;
if (bindings === undefined || ts.isNamespaceImport(bindings)) {
// This should be impossible. If a namespace import is present, the symbol was already
// considered imported above.
console.error(`Unexpected namespace import ${existingImportDeclaration.getText()}`);
const importClause = existingImportDeclaration.importClause;
if (importClause === undefined) {
return [[], ''];
}
let span = {start: importClause.getStart(), length: importClause.getWidth()};
const updatedBindings = updateImport(existingImportDeclaration, symbolName, importName);
if (updatedBindings === undefined) {
return [[], ''];
}
let span = {start: bindings.getStart(), length: bindings.getWidth()};
const updatedBindings = updateImport(bindings, symbolName, importName);
const importString = printNode(updatedBindings, file);
return [[{span, newText: importString}], importName];
}
Expand Down Expand Up @@ -561,6 +572,12 @@ export function isStandaloneDecorator(decorator: ts.Decorator): boolean | null {
* import {exportedSpecifierName as localName} from 'rawModuleSpecifier';
* ```
*
* If the component is exported by default, follows the format:
*
* ```
* import exportedSpecifierName from 'rawModuleSpecifier';
* ```
*
* If `exportedSpecifierName` is null, or is equal to `name`, then the qualified import alias will
* be omitted.
*/
Expand All @@ -575,13 +592,19 @@ export function generateImport(
}
const name = ts.factory.createIdentifier(localName);
const moduleSpec = ts.factory.createStringLiteral(rawModuleSpecifier);
let importClauseName: ts.Identifier | undefined;
let importBindings: ts.NamedImportBindings | undefined;

if (localName === 'default' && exportedSpecifierName !== null) {
importClauseName = ts.factory.createIdentifier(exportedSpecifierName);
} else {
importBindings = ts.factory.createNamedImports([
ts.factory.createImportSpecifier(false, propName, name),
]);
}
return ts.factory.createImportDeclaration(
undefined,
ts.factory.createImportClause(
false,
undefined,
ts.factory.createNamedImports([ts.factory.createImportSpecifier(false, propName, name)]),
),
ts.factory.createImportClause(false, importClauseName, importBindings),
moduleSpec,
undefined,
);
Expand All @@ -591,19 +614,47 @@ export function generateImport(
* Update an existing named import with a new member.
* If `exportedSpecifierName` is null, or is equal to `name`, then the qualified import alias will
* be omitted.
* If the `localName` is `default` and `exportedSpecifierName` is not null, the `exportedSpecifierName`
* is used as the default import name.
*/
export function updateImport(
imp: ts.NamedImports,
importDeclaration: ts.ImportDeclaration,
localName: string,
exportedSpecifierName: string | null,
): ts.NamedImports {
): ts.ImportClause | undefined {
const importClause = importDeclaration.importClause;
if (importClause === undefined) {
return undefined;
}
const bindings = importClause.namedBindings;
if (bindings !== undefined && ts.isNamespaceImport(bindings)) {
// This should be impossible. If a namespace import is present, the symbol was already
// considered imported above.
console.error(`Unexpected namespace import ${importDeclaration.getText()}`);
return undefined;
}
if (localName === 'default' && exportedSpecifierName !== null) {
const importClauseName = ts.factory.createIdentifier(exportedSpecifierName);
return ts.factory.updateImportClause(
importClause,
false,
importClauseName,
importClause.namedBindings,
);
}
let propertyName: ts.Identifier | undefined;
if (exportedSpecifierName !== null && exportedSpecifierName !== localName) {
propertyName = ts.factory.createIdentifier(exportedSpecifierName);
}
const name = ts.factory.createIdentifier(localName);
const newImport = ts.factory.createImportSpecifier(false, propertyName, name);
return ts.factory.updateNamedImports(imp, [...imp.elements, newImport]);
let namedImport: ts.NamedImports;
if (bindings === undefined) {
namedImport = ts.factory.createNamedImports([newImport]);
} else {
namedImport = ts.factory.updateNamedImports(bindings, [...bindings.elements, newImport]);
}
return ts.factory.updateImportClause(importClause, false, importClause.name, namedImport);
}

let printer: ts.Printer | null = null;
Expand Down Expand Up @@ -639,6 +690,8 @@ export function getCodeActionToImportTheDirectiveDeclaration(
const potentialImports = compiler
.getTemplateTypeChecker()
.getPotentialImportsFor(directive.ref, importOn, PotentialImportMode.Normal);
const declarationName = directive.ref.node.name.getText();

for (const potentialImport of potentialImports) {
const fileImportChanges: ts.TextChange[] = [];
let importName: string;
Expand All @@ -649,6 +702,7 @@ export function getCodeActionToImportTheDirectiveDeclaration(
compiler.getCurrentProgram().getTypeChecker(),
importOn.getSourceFile(),
potentialImport.symbolName,
declarationName,
potentialImport.moduleSpecifier,
currMatchSymbol.getSourceFile(),
);
Expand All @@ -662,6 +716,7 @@ export function getCodeActionToImportTheDirectiveDeclaration(
compiler.getCurrentProgram().getTypeChecker(),
importOn.getSourceFile(),
'forwardRef',
declarationName,
'@angular/core',
importOn.getSourceFile(),
);
Expand Down
117 changes: 117 additions & 0 deletions packages/language-service/test/code_fixes_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,123 @@ describe('code fixes', () => {
[``, `, imports: [Bar2Module]`],
]);
});

it('for a default exported component', () => {
const standaloneFiles = {
'foo.ts': `
import {Component} from '@angular/core';
@Component({
selector: 'foo',
template: '<bar></bar>',
standalone: true
})
export class FooComponent {}
`,
'bar.ts': `
import {Component} from '@angular/core';
@Component({
selector: 'bar',
template: '<div>bar</div>',
standalone: true
})
class BarComponent {}
export default BarComponent;
`,
};

const project = createModuleAndProjectWithDeclarations(env, 'test', {}, {}, standaloneFiles);
const diags = project.getDiagnosticsForFile('foo.ts');
const fixFile = project.openFile('foo.ts');
fixFile.moveCursorToText('<¦bar>');

const codeActions = project.getCodeFixesAtPosition('foo.ts', fixFile.cursor, fixFile.cursor, [
diags[0].code,
]);
const actionChanges = allChangesForCodeActions(fixFile.contents, codeActions);
actionChangesMatch(actionChanges, `Import BarComponent from './bar' on FooComponent`, [
[``, `import BarComponent from "./bar";`],
[``, `, imports: [BarComponent]`],
]);
});

it('for a default exported component and reuse the existing import declarations', () => {
const standaloneFiles = {
'foo.ts': `
import {Component} from '@angular/core';
import {test} from './bar';
@Component({
selector: 'foo',
template: '<bar></bar>',
standalone: true
})
export class FooComponent {}
`,
'bar.ts': `
import {Component} from '@angular/core';
@Component({
selector: 'bar',
template: '<div>bar</div>',
standalone: true
})
class BarComponent {}
export default BarComponent;
export const test = 1;
`,
};

const project = createModuleAndProjectWithDeclarations(env, 'test', {}, {}, standaloneFiles);
const diags = project.getDiagnosticsForFile('foo.ts');
const fixFile = project.openFile('foo.ts');
fixFile.moveCursorToText('<¦bar>');

const codeActions = project.getCodeFixesAtPosition('foo.ts', fixFile.cursor, fixFile.cursor, [
diags[0].code,
]);
const actionChanges = allChangesForCodeActions(fixFile.contents, codeActions);
actionChangesMatch(actionChanges, `Import BarComponent from './bar' on FooComponent`, [
[`{te`, `BarComponent, { test }`],
[``, `, imports: [BarComponent]`],
]);
});

it('for a default exported component and reuse the existing imported component name', () => {
const standaloneFiles = {
'foo.ts': `
import {Component} from '@angular/core';
import NewBarComponent, {test} from './bar';
@Component({
selector: 'foo',
template: '<bar></bar>',
standalone: true
})
export class FooComponent {}
`,
'bar.ts': `
import {Component} from '@angular/core';
@Component({
selector: 'bar',
template: '<div>bar</div>',
standalone: true
})
class BarComponent {}
export default BarComponent;
export const test = 1;
`,
};

const project = createModuleAndProjectWithDeclarations(env, 'test', {}, {}, standaloneFiles);
const diags = project.getDiagnosticsForFile('foo.ts');
const fixFile = project.openFile('foo.ts');
fixFile.moveCursorToText('<¦bar>');

const codeActions = project.getCodeFixesAtPosition('foo.ts', fixFile.cursor, fixFile.cursor, [
diags[0].code,
]);
const actionChanges = allChangesForCodeActions(fixFile.contents, codeActions);
actionChangesMatch(actionChanges, `Import NewBarComponent from './bar' on FooComponent`, [
[``, `, imports: [NewBarComponent]`],
]);
});
});
});

Expand Down
17 changes: 11 additions & 6 deletions packages/language-service/test/ts_utils_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,12 +215,17 @@ describe('TS util', () => {

it('updateImport', () => {
let imp = generateImport('Foo', null, './foo');
let namedImp = updateImport(imp.importClause!.namedBindings! as ts.NamedImports, 'Bar', null);
expect(print(namedImp)).toEqual(`{ Foo, Bar }`);
namedImp = updateImport(imp.importClause!.namedBindings! as ts.NamedImports, 'Foo_2', 'Foo');
expect(print(namedImp)).toEqual(`{ Foo, Foo as Foo_2 }`);
namedImp = updateImport(imp.importClause!.namedBindings! as ts.NamedImports, 'Bar', 'Bar');
expect(print(namedImp)).toEqual(`{ Foo, Bar }`);
let namedImp = updateImport(imp, 'Bar', null);
expect(print(namedImp!)).toEqual(`{ Foo, Bar }`);
namedImp = updateImport(imp, 'Foo_2', 'Foo');
expect(print(namedImp!)).toEqual(`{ Foo, Foo as Foo_2 }`);
namedImp = updateImport(imp, 'Bar', 'Bar');
expect(print(namedImp!)).toEqual(`{ Foo, Bar }`);
namedImp = updateImport(imp, 'default', 'Baz');
expect(print(namedImp!)).toEqual(`Baz, { Foo }`);
imp = generateImport('default', 'Foo', './foo');
namedImp = updateImport(imp, 'Bar', null);
expect(print(namedImp!)).toEqual(`Foo, { Bar }`);
});

it('nonCollidingImportName', () => {
Expand Down

0 comments on commit 67b2c33

Please sign in to comment.