-
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
JavaScript LS scaffolding + JS module inference #5266
Conversation
? moduleName !== "require" | ||
: i === 1 | ||
? moduleName !== "exports" | ||
: i !== 2 || moduleName !== "module"; |
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.
Consider extracting this out into a function.
function shouldRecordAmdDefineName(index: number, moduleName: string) {
// record first item in the list only if its name is not "require"
// record second item in the list only if its name is not "exports"
// record third item in the list only if its name is not "module"
// record all other items in the list unconditionally
switch (index) {
case 0:
return moduleName !== "require";
case 1:
return moduleName !== "exports";
}
return index !== 2 || moduleName !== "module";
}
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.
We can remove the AMD logic entirely
return links.type = checkExpression((<BinaryExpression>declaration).right); | ||
} | ||
// Handle exports.p = expr | ||
if (declaration.kind === SyntaxKind.PropertyAccessExpression && declaration.parent.kind === SyntaxKind.BinaryExpression) { |
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.
Don't think you need the declaration.parent.kind
test. There should never be a case where that isn't true.
bindAnonymousDeclaration(file, SymbolFlags.ValueModule, `"${removeFileExtension(file.fileName) }"`); | ||
} | ||
|
||
function bindExportAssignment(node: ExportAssignment|BinaryExpression) { |
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.
We usually put a blank on both sides of the |
operator.
@vladima can you address the comments in program.ts? |
# Conflicts: # tests/webTestServer.ts
# Conflicts: # lib/lib.es6.d.ts # lib/tsc.js # lib/tsserver.js # lib/typescript.d.ts # lib/typescript.js # lib/typescriptServices.d.ts # lib/typescriptServices.js # src/compiler/binder.ts # src/compiler/checker.ts # src/compiler/parser.ts # src/compiler/program.ts # src/harness/fourslash.ts
# Conflicts: # src/compiler/parser.ts
JavaScript LS scaffolding + JS module inference
This is a subset of the prior PR that carves out only the code we need to get a) JavaScript intellisense in general and b) CommonJS module inference in JavaScript files.
Includes work by @vladima in 'services' to get the correct compilation context to Roslyn