-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Relax import/export elision rules for separate compilation #2550
Changes from 3 commits
a51f0bf
d2d3f1e
8f616ce
4b7e6cf
a6c88e2
955b4c0
c885f59
49b3f85
1803d73
1bdcaa3
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 |
---|---|---|
|
@@ -1587,6 +1587,7 @@ module ts { | |
target?: ScriptTarget; | ||
version?: boolean; | ||
watch?: boolean; | ||
separateCompilation?: boolean; | ||
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. i think we need to make use of const enums from an ambient declaration an error while using this module, also consider using of internal modules on the global scope as an error.. also we need to make that available on the command line, as an experimental flag. |
||
/* @internal */ stripInternal?: boolean; | ||
[option: string]: string | number | boolean; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1635,6 +1635,77 @@ module ts { | |
sourceFile.scriptSnapshot = scriptSnapshot; | ||
} | ||
|
||
export function transpile(input: string, compilerOptions?: CompilerOptions, fileName?: string, syntaxErrors?: Diagnostic[]): string { | ||
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. We need some comments on what this function is about and the constraints of using it. 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. done |
||
let options = compilerOptions ? ts.clone(compilerOptions) : getDefaultCompilerOptions(); | ||
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. u do not need the ts. prefix |
||
|
||
// Single file transformation is only guaranteed to be correct inside an external module. | ||
// External modules have their own scope and cannot contribute to internal modules outside their | ||
// scope. | ||
if (options.target !== ScriptTarget.ES6 && (!options.module || options.module === ModuleKind.None)) { | ||
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. We need to check that whatever we parse is an external module as well. else it should be an error. |
||
// TODO: add errors | ||
} | ||
|
||
// In single file transformation the compiler does not have access to declaration sites. | ||
// Const enum property accesses will not be detected if they are not in the same file as the enum. | ||
// thus they are going to be emitted as normal propety access. To ensure correct behaviour at runtime, | ||
// we need to generate the actual enum object so that the property accesses do not fail. | ||
options.preserveConstEnums = true; | ||
|
||
// No reason to get declarations, we are only returning javascript | ||
options.declaration = false; | ||
|
||
// Filename can be non-ts file. | ||
options.allowNonTsExtensions = true; | ||
|
||
// enable relaxed emit rules | ||
options.separateCompilation = true; | ||
|
||
// We are not resolving references or modules, or even including the default library. Disabling | ||
// emit on error will block generating the output and has no meaningful use here. | ||
options.noEmitOnError = false; | ||
|
||
// No resolution requests will be honored anyways. So do not do it | ||
options.noResolve = true; | ||
|
||
// TODO (vladima): this should be enabled after adding support to inlinable source maps | ||
options.sourceMap = false; | ||
|
||
// Parse | ||
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. As per our discussion today, for transpile function: Now we need a good name for the flag :) 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. done except for the flag name |
||
var inputFileName = fileName || "module.ts"; | ||
var sourceFile = ts.createSourceFile(inputFileName, input, options.target); | ||
|
||
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. u do not need the ts |
||
// Store syntactic diagnostics | ||
if (syntaxErrors && sourceFile.parseDiagnostics) { | ||
syntaxErrors.push(...sourceFile.parseDiagnostics); | ||
} | ||
|
||
// Output | ||
let outputText: string; | ||
|
||
// Create a compilerHost object to allow the compiler to read and write files | ||
var compilerHost: CompilerHost = { | ||
getSourceFile: (fileName, target) => fileName === inputFileName ? sourceFile : undefined, | ||
writeFile: (name, text, writeByteOrderMark) => { | ||
Debug.assert(outputText === undefined, "Unexpected multiple outputs for the file: " + name); | ||
outputText = text; | ||
}, | ||
getDefaultLibFileName: () => "lib.d.ts", | ||
useCaseSensitiveFileNames: () => false, | ||
getCanonicalFileName: fileName => fileName, | ||
getCurrentDirectory: () => "", | ||
getNewLine: () => "\r\n" | ||
}; | ||
|
||
var program = ts.createProgram([inputFileName], options, compilerHost); | ||
|
||
// Emit | ||
program.emit(); | ||
|
||
Debug.assert(outputText !== undefined, "Output generation failed"); | ||
|
||
return outputText; | ||
} | ||
|
||
export function createLanguageServiceSourceFile(fileName: string, scriptSnapshot: IScriptSnapshot, scriptTarget: ScriptTarget, version: string, setNodeParents: boolean): SourceFile { | ||
let sourceFile = createSourceFile(fileName, scriptSnapshot.getText(0, scriptSnapshot.getLength()), scriptTarget, setNodeParents); | ||
setSourceFileFields(sourceFile, scriptSnapshot, version); | ||
|
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.
i think we can just make this the default behavior. the only cases we would be breaking are error cases, that in the past we elided modules imports, and now we will be writing them. is that correct?
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.
done
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.
actually I've re-checked - this might lead to cascading errors so I'll keep relaxed behavior specific to
separateCompilation
mode