Module import

Module import

Works with local modules and standard modules.

import iterator from '@iter';

module m {
  export var x = 42;
}

import {x: renamed} from m;

import * is done by associating the module expression with the tree during the
ImportStarVisitor phase. Once we get to the tranformation phase we get the
module symbol and we know what it exports.

import * from '@name';

BUG=http://code.google.com/p/traceur-compiler/issues/detail?id=26
TEST=feature/Modules/*

Committed: https://code.google.com/p/traceur-compiler/source/detail?r=f4f8788

[arv, 2012-04-15 22:54:12]

Hi Chris,

I'm not sure if you still are interested in this. I thought I would send it to
you since you started looking at this way back before JSConf :-)

It turns out this was a lot simpler than I ever imagined. We had most of the
infrastructure in place already. An import statement becomes a destructuring
variable statement. The 'import *' case was a bit trickier since we don't have
access to the module symbols during the transformation phase. To solve this I
gather the associations between the tree and the module symbol during the module
analyzer phase.

Happy traceuring.

[cburrows, 2012-04-16 06:04:56]

On 2012/04/15 22:54:12, arv-chromium wrote:
> Hi Chris,
> 
> I'm not sure if you still are interested in this. I thought I would send it to
> you since you started looking at this way back before JSConf :-)

Of course! I will take a look at this tomorrow or Tuesday.

[slightlylate, 2012-04-16 12:48:29]

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ModuleTra...
File src/codegeneration/ModuleTransformer.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ModuleTra...
src/codegeneration/ModuleTransformer.js:150: this.project = project;
is this meant to be this.project_?

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ModuleTra...
src/codegeneration/ModuleTransformer.js:199: // if identifier we need to output
the object pattern {id}
"If"

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ProgramTr...
File src/codegeneration/ProgramTransformer.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ProgramTr...
src/codegeneration/ProgramTransformer.js:235: return
ModuleTransformer.transformAsModule(this.project_, opt_module,
"this.project" or "this.project_"?

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/module/Mo...
File src/codegeneration/module/ModuleVisitor.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/module/Mo...
src/codegeneration/module/ModuleVisitor.js:24: var EXPORT_DECLARATION =
traceur.syntax.trees.ParseTreeType.EXPORT_DECLARATION;
hoist ParseTreeType to a var to shorten to 80

https://codereview.appspot.com/6033051/diff/1035/src/runtime/modules.js
File src/runtime/modules.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/runtime/modules.js#newco...
src/runtime/modules.js:513: var $hasOwn = Object.prototype.hasOwnProperty;
unused?

https://codereview.appspot.com/6033051/diff/1035/src/semantics/ModuleAnalyzer.js
File src/semantics/ModuleAnalyzer.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/semantics/ModuleAnalyzer...
src/semantics/ModuleAnalyzer.js:20: var ModuleDeclarationVisitor =
traceur.codegeneration.module.ModuleDeclarationVisitor;
80

https://codereview.appspot.com/6033051/diff/1035/src/semantics/symbols/Projec...
File src/semantics/symbols/Project.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/semantics/symbols/Projec...
src/semantics/symbols/Project.js:37: var standardUrlRe = /^@\w+$/;
URL or module?

[johnjbarton, 2012-04-16 16:48:41]

Cool, now I am anxious to try this out.

https://codereview.appspot.com/6033051/diff/1035/demo/repl.html
File demo/repl.html (right):

https://codereview.appspot.com/6033051/diff/1035/demo/repl.html#newcode152
demo/repl.html:152: ErrorReporter.format(location, format, args) + '\n';
I guess this change is orthogonal to the module stuff. We might be happier in
the long run if we have such changes in two commits.

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ModuleTra...
File src/codegeneration/ModuleTransformer.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ModuleTra...
src/codegeneration/ModuleTransformer.js:148: function ModuleTransformer(project)
{
@param {Project} project, needed by ModuleVisitor to get modules by URL

[arv, 2012-04-16 16:53:23]

https://codereview.appspot.com/6033051/diff/1035/demo/repl.html
File demo/repl.html (right):

https://codereview.appspot.com/6033051/diff/1035/demo/repl.html#newcode152
demo/repl.html:152: ErrorReporter.format(location, format, args) + '\n';
On 2012/04/16 16:48:42, johnjbarton wrote:
> I guess this change is orthogonal to the module stuff. We might be happier in
> the long run if we have such changes in two commits.

Yeah. I got lazy. I'll split this into 3 changes.

1. ErrorReporter.format
2. Add support for expected errors
3. module import.

This change will be modules only after that.

[arv, 2012-04-17 04:18:22]

https://codereview.appspot.com/6033051/diff/1035/demo/repl.html
File demo/repl.html (right):

https://codereview.appspot.com/6033051/diff/1035/demo/repl.html#newcode152
demo/repl.html:152: ErrorReporter.format(location, format, args) + '\n';
On 2012/04/16 16:53:23, arv-chromium wrote:
> On 2012/04/16 16:48:42, johnjbarton wrote:
> > I guess this change is orthogonal to the module stuff. We might be happier
in
> > the long run if we have such changes in two commits.
> 
> Yeah. I got lazy. I'll split this into 3 changes.
> 
> 1. ErrorReporter.format
> 2. Add support for expected errors
> 3. module import.
> 
> This change will be modules only after that.

1. https://codereview.appspot.com/6057043/
2. https://codereview.appspot.com/6049045/
3. This review

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ModuleTra...
File src/codegeneration/ModuleTransformer.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ModuleTra...
src/codegeneration/ModuleTransformer.js:148: function ModuleTransformer(project)
{
On 2012/04/16 16:48:42, johnjbarton wrote:
> @param {Project} project, needed by ModuleVisitor to get modules by URL

Done.

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ModuleTra...
src/codegeneration/ModuleTransformer.js:150: this.project = project;
On 2012/04/16 12:48:29, slightlylate wrote:
> is this meant to be this.project_?

I wasn't sure if I wanted to make project public or not. I made it "private" in
this case.

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ModuleTra...
src/codegeneration/ModuleTransformer.js:199: // if identifier we need to output
the object pattern {id}
On 2012/04/16 12:48:29, slightlylate wrote:
> "If"

Done.

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ProgramTr...
File src/codegeneration/ProgramTransformer.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/ProgramTr...
src/codegeneration/ProgramTransformer.js:235: return
ModuleTransformer.transformAsModule(this.project_, opt_module,
On 2012/04/16 12:48:29, slightlylate wrote:
> "this.project" or "this.project_"?

This file is all .project_. I'm trying to stick to project_ for now.

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/module/Mo...
File src/codegeneration/module/ModuleVisitor.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/module/Mo...
src/codegeneration/module/ModuleVisitor.js:24: var EXPORT_DECLARATION =
traceur.syntax.trees.ParseTreeType.EXPORT_DECLARATION;
On 2012/04/16 12:48:29, slightlylate wrote:
> hoist ParseTreeType to a var to shorten to 80

I haven't bother with > 80 chars for these imports.

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/module/Mo...
src/codegeneration/module/ModuleVisitor.js:24: var EXPORT_DECLARATION =
traceur.syntax.trees.ParseTreeType.EXPORT_DECLARATION;
On 2012/04/16 12:48:29, slightlylate wrote:
> hoist ParseTreeType to a var to shorten to 80

I removed these "imports" in favor of a using ParseTreeType.FOO.

https://codereview.appspot.com/6033051/diff/1035/src/codegeneration/module/Mo...
src/codegeneration/module/ModuleVisitor.js:45: this.project = project;
This one needs to be public. Subclasses uses it.

https://codereview.appspot.com/6033051/diff/1035/src/runtime/modules.js
File src/runtime/modules.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/runtime/modules.js#newco...
src/runtime/modules.js:513: var $hasOwn = Object.prototype.hasOwnProperty;
On 2012/04/16 12:48:29, slightlylate wrote:
> unused?

Done.

https://codereview.appspot.com/6033051/diff/1035/src/semantics/ModuleAnalyzer.js
File src/semantics/ModuleAnalyzer.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/semantics/ModuleAnalyzer...
src/semantics/ModuleAnalyzer.js:20: var ModuleDeclarationVisitor =
traceur.codegeneration.module.ModuleDeclarationVisitor;
On 2012/04/16 12:48:29, slightlylate wrote:
> 80

Ignored

https://codereview.appspot.com/6033051/diff/1035/src/semantics/symbols/Projec...
File src/semantics/symbols/Project.js (right):

https://codereview.appspot.com/6033051/diff/1035/src/semantics/symbols/Projec...
src/semantics/symbols/Project.js:37: var standardUrlRe = /^@\w+$/;
On 2012/04/16 12:48:29, slightlylate wrote:
> URL or module?

Renamed to standardModuleUrlRegExp.

[slightlylate, 2012-04-17 11:45:27]

LGTM

https://codereview.appspot.com/6033051/diff/9001/src/codegeneration/ModuleTra...
File src/codegeneration/ModuleTransformer.js (right):

https://codereview.appspot.com/6033051/diff/9001/src/codegeneration/ModuleTra...
src/codegeneration/ModuleTransformer.js:70: function toBindingIdentifier(tree) {
Seems strange to me that this isn't a method of the tree object?

[arv, 2012-04-17 16:33:49]

https://codereview.appspot.com/6033051/diff/9001/src/codegeneration/ModuleTra...
File src/codegeneration/ModuleTransformer.js (right):

https://codereview.appspot.com/6033051/diff/9001/src/codegeneration/ModuleTra...
src/codegeneration/ModuleTransformer.js:70: function toBindingIdentifier(tree) {
On 2012/04/17 11:45:27, slightlylate wrote:
> Seems strange to me that this isn't a method of the tree object?

Agreed. ParseTree objects don't have methods. I think another option would be to
move it to ParseTreeFactory at least.