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

Compilation of Js Files #5471

Merged
merged 105 commits into from Nov 20, 2015

Conversation

Projects
None yet
5 participants
@sheetalkamat
Member

sheetalkamat commented Oct 30, 2015

Changes same as #5345 with few modifications and against master
Here is the list of items this PR supports:

  • Allow javascript files to be compiled on command line or through references/imports
  • tsconfig.json can take javascript files as file names to be compiled
  • If filenames are not specified in tsconfig.json, javascript files in the folder and not part of compilation
  • compiler options now have a flag named "allowJs" which when true will pick up javascript files in the folder for compilation if tscconfig.json doesn't supply file name list
  • Moved reporting of usage of typescript syntax in javascript file from services to compiler
  • Reports error if output is overwriting any of the input files (#4424)
  • Reports error if output is not unique location and multiple input files overwrite same output file (#4424)
  • .ts, .tsx and .d.ts files take precedence over .js files when creating list of files for compilation
  • Project runner options cleanup
  • Project runner to accept tsconfig.json tests
  • Bundle js files into --out and copy into --outDir
  • Transpilation of javascript files
  • JavaScript files do not emit declaration files and it is error to specify --allowJs and --declaration together

sheetalkamat added some commits Sep 9, 2015

Verify and fix scenario when .js and .ts files with same name are pre…
…sent and tsconfig doesnt specify any filenames
Add option --jsExtensions to handle extensions to treat as javascript
- Command line now takes --jsExtension multiple times or comma separated list of extensions
- tsconfig accepts array of extension strings

sheetalkamat added some commits Nov 12, 2015

Simplified logic of getting files if files werent suppplied in tsccon…
…fig.json

Since we dont support arbitary list of extensions to treat as .js,
it becomes easier to simplify code based on the assumptions
@mhegazy

This comment has been minimized.

Contributor

mhegazy commented on src/compiler/commandLineParser.ts in 0b21540 Nov 12, 2015

what about jsx?

This comment has been minimized.

Member

sheetalkamat replied Nov 12, 2015

Yep. Sorry missed that, will add it

@@ -12033,7 +12034,11 @@ namespace ts {
const symbol = getSymbolOfNode(node);
const localSymbol = node.localSymbol || symbol;
const firstDeclaration = getDeclarationOfKind(localSymbol, node.kind);
const firstDeclaration = forEach(symbol.declarations,

This comment has been minimized.

@vladima

vladima Nov 16, 2015

Contributor

can you please add a comment

  • why we are not picking the first declaration if it originates in javascript file
  • why we are using symbol instead of localSymbol
    ?
const start = new Date().getTime();
host = host || createCompilerHost(options);
// Map storing if there is emit blocking diagnostics for given input
const hasEmitBlockingDiagnostics = createFileMap<boolean>(!host.useCaseSensitiveFileNames() ? key => key.toLocaleLowerCase() : undefined);

This comment has been minimized.

@vladima

vladima Nov 16, 2015

Contributor

use ts.createGetCanonicalFileName

}
else {
const sourceFiles = targetSourceFile === undefined ? host.getSourceFiles() : [targetSourceFile];
forEach(sourceFiles, sourceFile => {

This comment has been minimized.

@vladima

vladima Nov 16, 2015

Contributor

for..of pls

const bundledSources = filter(host.getSourceFiles(),
sourceFile => !isDeclarationFile(sourceFile) && // Not a declaration file
(!isExternalModule(sourceFile) || // non module file
(getEmitModuleKind(options) && isExternalModule(sourceFile)))); // module that can emit

This comment has been minimized.

@vladima

vladima Nov 16, 2015

Contributor

using result of getEmitModuleKind directly without comparing it with something looks strange, can you please add a comment that will state that "falsy" value returned from this function denotes module kind that should not be emitted

declarationFilePath: string;
}
export function forEachExpectedEmitFile(host: EmitHost,

This comment has been minimized.

@vladima

vladima Nov 16, 2015

Contributor

I think this function needs a better name. To me forEach implies that this function will

  • have a callback parameter and this callback will accept one file as an argument
  • this callback will be invoked for every file in the list (probably module some filtering).
    For this function the only part that is true - it indeed accepts callback.

This comment has been minimized.

@sheetalkamat

sheetalkamat Nov 18, 2015

Member

I like the name because even if it doesn't follow the forEach type of callback the name explicitly suggests that the callback would be called for each emit file. If you have suggestion on name I can update accordingly.

let addedGlobalFileReference = false;
let allSourcesModuleElementDeclarationEmitInfo: ModuleElementDeclarationEmitInfo[] = [];
forEach(sourceFiles, sourceFile => {
if (!isSourceFileJavaScript(sourceFile)) {

This comment has been minimized.

@vladima

vladima Nov 16, 2015

Contributor

can you please invert the condition and return in then statement to reduce nesting

getDirectoryPath(normalizeSlashes(declarationFilePath)),
declFileName,
host.getCurrentDirectory(),
host.getCanonicalFileName,

This comment has been minimized.

@vladima

vladima Nov 16, 2015

Contributor

lost this, use arrow function instead

This comment has been minimized.

@sheetalkamat

sheetalkamat Nov 17, 2015

Member

But I am not using this?

This comment has been minimized.

@vladima

vladima Nov 17, 2015

Contributor

ok, sorry, I thought it was CompilerHost not EmitHost. There are users who use classes to implement host interfaces and for them losing this can be problematic. I've seen such implementations for CompilerHost, maybe it is less critical for EmitHost since we use this interface for internal decomposition, not as extensibility point for external customers

This comment has been minimized.

@mihailik

mihailik Nov 20, 2015

Contributor

A short comment in code would help though!

if (emitFileName) {
const emitFilePath = toPath(emitFileName, currentDirectory, getCanonicalFileName);
// Report error if the output overwrites input file
if (forEach(files, file => toPath(file.fileName, currentDirectory, getCanonicalFileName) === emitFilePath)) {

This comment has been minimized.

@vladima

vladima Nov 16, 2015

Contributor

program should have filesByName which is FileMap<SourceFile> so you can just check filesByName.contains

}
}
function createEmitBlockingDiagnostics(emitFileName: string, message: DiagnosticMessage) {

This comment has been minimized.

@vladima

vladima Nov 16, 2015

Contributor

since you've already computed path on the caller side, can you pass it here as a parameter?

@@ -1,108 +0,0 @@
tests/cases/compiler/classdecl.ts(12,16): error TS1056: Accessors are only available when targeting ECMAScript 5 and higher.

This comment has been minimized.

@vladima

vladima Nov 16, 2015

Contributor

can you please clarify why this file is no longer an error?

This comment has been minimized.

@sheetalkamat

sheetalkamat Nov 16, 2015

Member

Because it was missing @target: es5 in the test cases I added it.

This comment has been minimized.

@vladima

vladima Nov 17, 2015

Contributor

got it

@vladima

This comment has been minimized.

Contributor

vladima commented Nov 17, 2015

looks good modulo few comments

sheetalkamat added a commit that referenced this pull request Nov 20, 2015

@sheetalkamat sheetalkamat merged commit 883b8d9 into master Nov 20, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@sheetalkamat sheetalkamat deleted the jsFileCompilation branch Nov 20, 2015

@Microsoft Microsoft locked and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.