-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
treat ambient non-aliased 'require' as commonjs 'require' #11819
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12819,16 +12819,41 @@ namespace ts { | |
} | ||
|
||
// In JavaScript files, calls to any identifier 'require' are treated as external module imports | ||
if (isInJavaScriptFile(node) && | ||
isRequireCall(node, /*checkArgumentIsStringLiteral*/true) && | ||
// Make sure require is not a local function | ||
!resolveName(node.expression, (<Identifier>node.expression).text, SymbolFlags.Value, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined)) { | ||
if (isInJavaScriptFile(node) && isCommonJsRequire(node)) { | ||
return resolveExternalModuleTypeByLiteral(<StringLiteral>node.arguments[0]); | ||
} | ||
|
||
return getReturnTypeOfSignature(signature); | ||
} | ||
|
||
function isCommonJsRequire(node: Node) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May be we need to use this function in completions.ts to determine whether to give module names as completion list? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May be we need to use this function in completions.ts to determine whether to give module names as completion list? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. certainly but it is not related to the original issue so I'd put it into different PR |
||
if (!isRequireCall(node, /*checkArgumentIsStringLiteral*/true)) { | ||
return false; | ||
} | ||
// Make sure require is not a local function | ||
const resolvedRequire = resolveName(node.expression, (<Identifier>node.expression).text, SymbolFlags.Value, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined); | ||
if (!resolvedRequire) { | ||
// project does not contain symbol named 'require' - assume commonjs require | ||
return true; | ||
} | ||
// project includes symbol named 'require' - make sure that it it ambient and local non-alias | ||
if (resolvedRequire.flags & SymbolFlags.Alias) { | ||
return false; | ||
} | ||
|
||
const targetDeclarationKind = resolvedRequire.flags & SymbolFlags.Function | ||
? SyntaxKind.FunctionDeclaration | ||
: resolvedRequire.flags & SymbolFlags.Variable | ||
? SyntaxKind.VariableDeclaration | ||
: SyntaxKind.Unknown; | ||
if (targetDeclarationKind !== SyntaxKind.Unknown) { | ||
const decl = getDeclarationOfKind(resolvedRequire, targetDeclarationKind); | ||
// function/variable declaration should be ambient | ||
return isInAmbientContext(decl); | ||
} | ||
return false; | ||
} | ||
|
||
function checkTaggedTemplateExpression(node: TaggedTemplateExpression): Type { | ||
return getReturnTypeOfSignature(getResolvedSignature(node)); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
//// [tests/cases/compiler/ambientRequireFunction.ts] //// | ||
|
||
//// [node.d.ts] | ||
|
||
|
||
declare function require(moduleName: string): any; | ||
|
||
declare module "fs" { | ||
export function readFileSync(s: string): string; | ||
} | ||
|
||
//// [app.js] | ||
/// <reference path="node.d.ts"/> | ||
|
||
const fs = require("fs"); | ||
const text = fs.readFileSync("/a/b/c"); | ||
|
||
//// [app.js] | ||
/// <reference path="node.d.ts"/> | ||
var fs = require("fs"); | ||
var text = fs.readFileSync("/a/b/c"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
=== tests/cases/compiler/app.js === | ||
/// <reference path="node.d.ts"/> | ||
|
||
const fs = require("fs"); | ||
>fs : Symbol(fs, Decl(app.js, 2, 5)) | ||
>require : Symbol(require, Decl(node.d.ts, 0, 0)) | ||
>"fs" : Symbol("fs", Decl(node.d.ts, 2, 50)) | ||
|
||
const text = fs.readFileSync("/a/b/c"); | ||
>text : Symbol(text, Decl(app.js, 3, 5)) | ||
>fs.readFileSync : Symbol(readFileSync, Decl(node.d.ts, 4, 21)) | ||
>fs : Symbol(fs, Decl(app.js, 2, 5)) | ||
>readFileSync : Symbol(readFileSync, Decl(node.d.ts, 4, 21)) | ||
|
||
=== tests/cases/compiler/node.d.ts === | ||
|
||
|
||
declare function require(moduleName: string): any; | ||
>require : Symbol(require, Decl(node.d.ts, 0, 0)) | ||
>moduleName : Symbol(moduleName, Decl(node.d.ts, 2, 25)) | ||
|
||
declare module "fs" { | ||
export function readFileSync(s: string): string; | ||
>readFileSync : Symbol(readFileSync, Decl(node.d.ts, 4, 21)) | ||
>s : Symbol(s, Decl(node.d.ts, 5, 33)) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
=== tests/cases/compiler/app.js === | ||
/// <reference path="node.d.ts"/> | ||
|
||
const fs = require("fs"); | ||
>fs : typeof "fs" | ||
>require("fs") : typeof "fs" | ||
>require : (moduleName: string) => any | ||
>"fs" : "fs" | ||
|
||
const text = fs.readFileSync("/a/b/c"); | ||
>text : string | ||
>fs.readFileSync("/a/b/c") : string | ||
>fs.readFileSync : (s: string) => string | ||
>fs : typeof "fs" | ||
>readFileSync : (s: string) => string | ||
>"/a/b/c" : "/a/b/c" | ||
|
||
=== tests/cases/compiler/node.d.ts === | ||
|
||
|
||
declare function require(moduleName: string): any; | ||
>require : (moduleName: string) => any | ||
>moduleName : string | ||
|
||
declare module "fs" { | ||
export function readFileSync(s: string): string; | ||
>readFileSync : (s: string) => string | ||
>s : string | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
//// [app.js] | ||
|
||
function require(a) { | ||
return a; | ||
} | ||
|
||
const fs = require("fs"); | ||
const text = fs.readFileSync("/a/b/c"); | ||
|
||
//// [app.js] | ||
function require(a) { | ||
return a; | ||
} | ||
var fs = require("fs"); | ||
var text = fs.readFileSync("/a/b/c"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
=== tests/cases/compiler/app.js === | ||
|
||
function require(a) { | ||
>require : Symbol(require, Decl(app.js, 0, 0)) | ||
>a : Symbol(a, Decl(app.js, 1, 17)) | ||
|
||
return a; | ||
>a : Symbol(a, Decl(app.js, 1, 17)) | ||
} | ||
|
||
const fs = require("fs"); | ||
>fs : Symbol(fs, Decl(app.js, 5, 5)) | ||
>require : Symbol(require, Decl(app.js, 0, 0)) | ||
|
||
const text = fs.readFileSync("/a/b/c"); | ||
>text : Symbol(text, Decl(app.js, 6, 5)) | ||
>fs : Symbol(fs, Decl(app.js, 5, 5)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
=== tests/cases/compiler/app.js === | ||
|
||
function require(a) { | ||
>require : (a: any) => any | ||
>a : any | ||
|
||
return a; | ||
>a : any | ||
} | ||
|
||
const fs = require("fs"); | ||
>fs : any | ||
>require("fs") : any | ||
>require : (a: any) => any | ||
>"fs" : "fs" | ||
|
||
const text = fs.readFileSync("/a/b/c"); | ||
>text : any | ||
>fs.readFileSync("/a/b/c") : any | ||
>fs.readFileSync : any | ||
>fs : any | ||
>readFileSync : any | ||
>"/a/b/c" : "/a/b/c" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// @module: commonjs | ||
// @allowJs: true | ||
// @outDir: ./out/ | ||
|
||
// @filename: node.d.ts | ||
|
||
declare function require(moduleName: string): any; | ||
|
||
declare module "fs" { | ||
export function readFileSync(s: string): string; | ||
} | ||
|
||
// @filename: app.js | ||
/// <reference path="node.d.ts"/> | ||
|
||
const fs = require("fs"); | ||
const text = fs.readFileSync("/a/b/c"); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
// @module: commonjs | ||
// @allowJs: true | ||
// @outDir: ./out/ | ||
|
||
// @filename: app.js | ||
function require(a) { | ||
return a; | ||
} | ||
|
||
const fs = require("fs"); | ||
const text = fs.readFileSync("/a/b/c"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be we need to use this function in completions.ts to determine whether to give module names as completion list?