Skip to content

Commit

Permalink
CommonJS imports support destructuring+property access (#40702)
Browse files Browse the repository at this point in the history
* CommonJS imports support destructuring+property access

Fixes #40578 for prettier

* will I ever remember semicolons? haha no

* move code around

* move function declaration closer to use

* Add missing space after `if`

Thanks to @weswigham for noticing this. Somehow it passed the linter.
  • Loading branch information
sandersn committed Sep 24, 2020
1 parent e6fdcce commit eac75f3
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 51 deletions.
39 changes: 24 additions & 15 deletions src/compiler/checker.ts
Expand Up @@ -2448,10 +2448,11 @@ namespace ts {
}

function getTargetOfImportEqualsDeclaration(node: ImportEqualsDeclaration | VariableDeclaration, dontResolveAlias: boolean): Symbol | undefined {
if (isVariableDeclaration(node) && node.initializer && isPropertyAccessExpression(node.initializer)) {
const name = (getLeftmostAccessExpression(node.initializer.expression) as CallExpression).arguments[0] as StringLiteral;
return isIdentifier(node.initializer.name)
? resolveSymbol(getPropertyOfType(resolveExternalModuleTypeByLiteral(name), node.initializer.name.escapedText))
const commonJSPropertyAccess = getCommonJSPropertyAccess(node);
if (commonJSPropertyAccess) {
const name = (getLeftmostAccessExpression(commonJSPropertyAccess.expression) as CallExpression).arguments[0] as StringLiteral;
return isIdentifier(commonJSPropertyAccess.name)
? resolveSymbol(getPropertyOfType(resolveExternalModuleTypeByLiteral(name), commonJSPropertyAccess.name.escapedText))
: undefined;
}
if (isVariableDeclaration(node) || node.moduleReference.kind === SyntaxKind.ExternalModuleReference) {
Expand Down Expand Up @@ -2646,12 +2647,8 @@ namespace ts {
return result;
}

function getExportOfModule(symbol: Symbol, specifier: ImportOrExportSpecifier | BindingElement, dontResolveAlias: boolean): Symbol | undefined {
function getExportOfModule(symbol: Symbol, name: Identifier, specifier: Declaration, dontResolveAlias: boolean): Symbol | undefined {
if (symbol.flags & SymbolFlags.Module) {
const name = specifier.propertyName ?? specifier.name;
if (!isIdentifier(name)) {
return undefined;
}
const exportSymbol = getExportsOfSymbol(symbol).get(name.escapedText);
const resolved = resolveSymbol(exportSymbol, dontResolveAlias);
markSymbolOfAliasDeclarationIfTypeOnly(specifier, exportSymbol, resolved, /*overwriteEmpty*/ false);
Expand All @@ -2668,10 +2665,10 @@ namespace ts {
}
}

function getExternalModuleMember(node: ImportDeclaration | ExportDeclaration | VariableDeclaration, specifier: ImportOrExportSpecifier | BindingElement, dontResolveAlias = false): Symbol | undefined {
function getExternalModuleMember(node: ImportDeclaration | ExportDeclaration | VariableDeclaration, specifier: ImportOrExportSpecifier | BindingElement | PropertyAccessExpression, dontResolveAlias = false): Symbol | undefined {
const moduleSpecifier = getExternalModuleRequireArgument(node) || (node as ImportDeclaration | ExportDeclaration).moduleSpecifier!;
const moduleSymbol = resolveExternalModuleName(node, moduleSpecifier)!; // TODO: GH#18217
const name = specifier.propertyName || specifier.name;
const name = !isPropertyAccessExpression(specifier) && specifier.propertyName || specifier.name;
if (!isIdentifier(name)) {
return undefined;
}
Expand All @@ -2691,10 +2688,10 @@ namespace ts {
else {
symbolFromVariable = getPropertyOfVariable(targetSymbol, name.escapedText);
}

// if symbolFromVariable is export - get its final target
symbolFromVariable = resolveSymbol(symbolFromVariable, dontResolveAlias);
let symbolFromModule = getExportOfModule(targetSymbol, specifier, dontResolveAlias);

let symbolFromModule = getExportOfModule(targetSymbol, name, specifier, dontResolveAlias);
if (symbolFromModule === undefined && name.escapedText === InternalSymbolName.Default) {
const file = find(moduleSymbol.declarations, isSourceFile);
if (canHaveSyntheticDefault(file, moduleSymbol, dontResolveAlias)) {
Expand Down Expand Up @@ -2782,11 +2779,23 @@ namespace ts {
}

function getTargetOfImportSpecifier(node: ImportSpecifier | BindingElement, dontResolveAlias: boolean): Symbol | undefined {
const resolved = getExternalModuleMember(isBindingElement(node) ? getRootDeclaration(node) as VariableDeclaration : node.parent.parent.parent, node, dontResolveAlias);
const root = isBindingElement(node) ? getRootDeclaration(node) as VariableDeclaration : node.parent.parent.parent;
const commonJSPropertyAccess = getCommonJSPropertyAccess(root);
const resolved = getExternalModuleMember(root, commonJSPropertyAccess || node, dontResolveAlias);
const name = node.propertyName || node.name;
if (commonJSPropertyAccess && resolved && isIdentifier(name)) {
return getPropertyOfType(getTypeOfSymbol(resolved), name.escapedText);
}
markSymbolOfAliasDeclarationIfTypeOnly(node, /*immediateTarget*/ undefined, resolved, /*overwriteEmpty*/ false);
return resolved;
}

function getCommonJSPropertyAccess(node: Node) {
if (isVariableDeclaration(node) && node.initializer && isPropertyAccessExpression(node.initializer)) {
return node.initializer;
}
}

function getTargetOfNamespaceExportDeclaration(node: NamespaceExportDeclaration, dontResolveAlias: boolean): Symbol {
const resolved = resolveExternalModuleSymbol(node.parent.symbol, dontResolveAlias);
markSymbolOfAliasDeclarationIfTypeOnly(node, /*immediateTarget*/ undefined, resolved, /*overwriteEmpty*/ false);
Expand Down Expand Up @@ -2940,7 +2949,7 @@ namespace ts {
finalTarget: Symbol | undefined,
overwriteEmpty: boolean,
): boolean {
if (!aliasDeclaration) return false;
if (!aliasDeclaration || isPropertyAccessExpression(aliasDeclaration)) return false;

// If the declaration itself is type-only, mark it and return.
// No need to check what it resolves to.
Expand Down
23 changes: 0 additions & 23 deletions tests/baselines/reference/jsDeclarationsTypeReferences2.errors.txt

This file was deleted.

2 changes: 1 addition & 1 deletion tests/baselines/reference/jsDeclarationsTypeReferences2.js
Expand Up @@ -38,4 +38,4 @@ export declare const o: {
m: number;
};
//// [index.d.ts]
export const thing: any;
export const thing: number;
24 changes: 12 additions & 12 deletions tests/baselines/reference/jsDeclarationsTypeReferences2.types
@@ -1,28 +1,28 @@
=== tests/cases/conformance/jsdoc/declarations/index.js ===
const{ a, m } = require("./something").o;
>a : any
>m : any
>a : number
>m : number
>require("./something").o : { a: number; m: number; }
>require("./something") : typeof import("tests/cases/conformance/jsdoc/declarations/something")
>require : any
>"./something" : "./something"
>o : { a: number; m: number; }

const thing = a + m
>thing : any
>a + m : any
>a : any
>m : any
>thing : number
>a + m : number
>a : number
>m : number

module.exports = {
>module.exports = { thing} : { thing: any; }
>module.exports : { thing: any; }
>module : { "\"tests/cases/conformance/jsdoc/declarations/index\"": { thing: any; }; }
>exports : { thing: any; }
>{ thing} : { thing: any; }
>module.exports = { thing} : { thing: number; }
>module.exports : { thing: number; }
>module : { "\"tests/cases/conformance/jsdoc/declarations/index\"": { thing: number; }; }
>exports : { thing: number; }
>{ thing} : { thing: number; }

thing
>thing : any
>thing : number

};

Expand Down

0 comments on commit eac75f3

Please sign in to comment.