Skip to content
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

Adding support for named AMD modules. #1158

Merged
merged 3 commits into from
Nov 18, 2014
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2096,7 +2096,11 @@ module ts {
function emitAMDModule(node: SourceFile, startIndex: number) {
var imports = getExternalImportDeclarations(node);
writeLine();
write("define([\"require\", \"exports\"");
write("define(");
if(node.amdModuleName) {
write("\"" + node.amdModuleName + "\", ");
}
write("[\"require\", \"exports\"");
forEach(imports, imp => {
write(", ");
emitLiteral(imp.externalModuleName);
Expand Down
12 changes: 11 additions & 1 deletion src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ module ts {
interface ReferenceComments {
referencedFiles: FileReference[];
amdDependencies: string[];
amdModuleName: string;
}

export function getSourceFileOfNode(node: Node): SourceFile {
Expand Down Expand Up @@ -4218,6 +4219,7 @@ module ts {
function processReferenceComments(): ReferenceComments {
var referencedFiles: FileReference[] = [];
var amdDependencies: string[] = [];
var amdModuleName: string;
commentRanges = [];
token = scanner.scan();

Expand All @@ -4237,6 +4239,12 @@ module ts {
}
}
else {
var amdModuleNameRegEx = /^\/\/\/\s*<amd-module\s+name\s*=\s*('|")(.+?)\1/gim;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would add a \s* before amd in "<amd-module"

and probably the same for amd-dependency

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not legal xml. Xml cannot have a space between the < and the start of the name token.

var amdModuleNameMatchResult = amdModuleNameRegEx.exec(comment);
if(amdModuleNameMatchResult) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Report error for duplicate entry if !amdModuleName?
Add test case for same

amdModuleName = amdModuleNameMatchResult[2];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mhegazy should we do any verification on amdModuleName user gave? Like it is all white spaces? all numbers? Should it always be identifier?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gisenberg We need test cases for amdModuleName irrespective of what @mhegazy comes back with.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is fine. user needs to opt in into this scheme, and i would consider it as the amd dependency, what you write is what you get.

}

var amdDependencyRegEx = /^\/\/\/\s*<amd-dependency\s+path\s*=\s*('|")(.+?)\1/gim;
var amdDependencyMatchResult = amdDependencyRegEx.exec(comment);
if (amdDependencyMatchResult) {
Expand All @@ -4247,7 +4255,8 @@ module ts {
commentRanges = undefined;
return {
referencedFiles: referencedFiles,
amdDependencies: amdDependencies
amdDependencies: amdDependencies,
amdModuleName: amdModuleName
};
}

Expand Down Expand Up @@ -4276,6 +4285,7 @@ module ts {
var referenceComments = processReferenceComments();
file.referencedFiles = referenceComments.referencedFiles;
file.amdDependencies = referenceComments.amdDependencies;
file.amdModuleName = referenceComments.amdModuleName;
file.statements = parseList(ParsingContext.SourceElements, /*checkForStrictMode*/ true, parseSourceElement);
file.externalModuleIndicator = getExternalModuleIndicator();
file.nodeCount = nodeCount;
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,7 @@ module ts {
getLineAndCharacterFromPosition(position: number): { line: number; character: number };
getPositionFromLineAndCharacter(line: number, character: number): number;
amdDependencies: string[];
amdModuleName: string;
referencedFiles: FileReference[];
syntacticErrors: Diagnostic[];
semanticErrors: Diagnostic[];
Expand Down
1 change: 1 addition & 0 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,7 @@ module ts {
public getLineAndCharacterFromPosition(position: number): { line: number; character: number } { return null; }
public getPositionFromLineAndCharacter(line: number, character: number): number { return -1; }
public amdDependencies: string[];
public amdModuleName: string;
public referencedFiles: FileReference[];
public syntacticErrors: Diagnostic[];
public semanticErrors: Diagnostic[];
Expand Down
22 changes: 22 additions & 0 deletions tests/baselines/reference/amdModuleName1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//// [amdModuleName1.ts]
///<amd-module name='NamedModule'/>
class Foo {
x: number;
constructor() {
this.x = 5;
}
}
export = Foo;


//// [amdModuleName1.js]
define("NamedModule", ["require", "exports"], function (require, exports) {
///<amd-module name='NamedModule'/>
var Foo = (function () {
function Foo() {
this.x = 5;
}
return Foo;
})();
return Foo;
});
19 changes: 19 additions & 0 deletions tests/baselines/reference/amdModuleName1.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
=== tests/cases/compiler/amdModuleName1.ts ===
///<amd-module name='NamedModule'/>
class Foo {
>Foo : Foo

x: number;
>x : number

constructor() {
this.x = 5;
>this.x = 5 : number
>this.x : number
>this : Foo
>x : number
}
}
export = Foo;
>Foo : Foo

9 changes: 9 additions & 0 deletions tests/cases/compiler/amdModuleName1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@module: amd
///<amd-module name='NamedModule'/>
class Foo {
x: number;
constructor() {
this.x = 5;
}
}
export = Foo;