consider es6 module syntax to be plain JS [fixes #3162]

[kieran]

per issue #3162

allows one to write es6 module import / export statements without backticks

tests included, comments welcome

[ghost]

👍

[gf3]

please

[michaelficarra]

This is really hacky and will break scoping. We will either add true support for module syntax or nothing at all.

[kieran]

@michaelficarra I'm not sure I understand what outcome you'd like to see here. Would you like to parse the entire import/export statement, then render it back 1:1?

Assignment symbols like * (as in import * from ...) make anything else impossible without the transpiler actually re-implementing JS's module loader, which is 904% outside the scope of CS.

[michaelficarra]

Would you like to parse the entire import/export statement, then render it back 1:1?

Yes.

Assignment symbols like * (as in import * from ...)

That's not part of ES6 for exactly this reason.

[gamaralf]

@michaelficarra I also do not understand why you think this is not a good idea. Using import in backticks already works!

I fail to see what is so wrong in adopting ES6's syntax for import/export.

[michaelficarra]

@gamaralf As I said, proper support of how imports affect scope is needed for this to be acceptable. It's not a syntax issue. This is just a hack that will cause worse problems.

[ghost]

@michaelficarra It would be helpful in this instance to have some examples of how imports would affect scope; I'm trying to think of some like if shadowing an existing variable or something.

[vendethiel]

import {a} from 'b'; a ?= 5 I guess

[michaelficarra]

Yep. We compile code (especially around assignment) differently based on whether references are in scope or not. Same thing with the compilation of a?, though that wouldn't break.

[kieran]

Can we re-open this PR since #3162 (comment) ?

[michaelficarra]

No, this is still a completely misguided implementation. We cannot just naively pass import/export declarations through to the output without understanding how they affect scope.

[kieran]

I'm not clear how the scope would be affected any differently than backticked JS passthroughs, which this implementation is modelled after.

Variables do not need to be declared beforehand, since import statements are implicit-declaration. The lexical scope would be maintained 1:1 with the output JS.

Maybe a failing test case would help me understand your point?

[lydell]

a = 1

The above compiles to:

var a;

a = 1;

Now take a look at this:

import a from 'something'

a = 1

With your patch, that would compile to:

import a from 'something';

var a;

a = 1;

That is, as far as I understand, invalid JavaScript. That would need to be compiled to:

import a from 'something';

a = 1;

That means that we have to track scope.

Also, your patch has several other problems.

If you accidentally make an error (which at least I do a lot), your patch will
happily accept them and output invalid JavaScript. For example:

import a from 'something

Ooops! Forgot the closing quote. But your patch would compile it to:

import a from 'something;

With your patch, it does not seem to be possible to break an import over several
lines, such as:

import {
    one,
    two,
    three,
} from 'utils'

Your patch does not handle imports in invalid locations either. The following
should be an error:

a = ->
  import b from 'something'

I suspect people would expect the following to work:

export fn = (args...) ->
  console.log args

I think I’m in favor of supporting that (but not export arr[0] = (arg) -> of course).

Finally, by parsing imports and exports we also allow tools like CoffeeLint to write linting rules for them. It also allows decaf, which constructs a real JavaScript AST instead of concatenating strings like CoffeeScript does, to output the imports and exports without having to write its own parser for them.

I hope the above will enlighten people why it matters to write a proper implementation with actual parsing and compiling, rather than sticking a quick hack into the code base.

Having worked on all parts of CoffeeScript myself, writing the code needed seems pretty easy. The hard parts are:

• Making sure that we’re following the ES2015 specification.
• Deciding how to test. Currently all tests work like this: We run some CoffeeScript code and then check that it did what it was supposed to. However, when it comes to import and export there is no JavaScript engine that can run that code yet. Do we have to resort to comparing the output code as a string with some expected string?

[michaelficarra]

Thank you @lydell. You've hit all the points I wanted to and more.

[kieran]

@lydell Thank you so much for that. These are all great examples of how this approach falls flat.

It seems that v8 is getting close to a working implementation (right now it only parses), so hopefully we'll have a reliable way to test the actual functionality soon, rather than string comparisons.

I'll take another stab at implementing this with those cases in mind.

Thanks again!

[lydell]

Another thing worth considering: Adding import in a file makes the whole file strict mode, right? Then we must assure that we don't output any code that is disallowed in strict mode. Not sure what the current status on that is in CoffeeScript.

[michaelficarra]

@lydell Not quite, it's whenever a file is treated as a Module, regardless of use of imports/exports. But you're right that it is a concern, as imports/exports can only exist in Modules, so the program must be strict mode. It's a tricky issue that we'll have to consider.

[GeoffreyBooth]

@lydell and @michaelficarra, I’ve been looking through the code, and the code in this PR, with the thought of trying to implement modules that satisfies all the criteria that @lydell lists above. Do you mind sharing some guidance as to where in the code you would recommend implementing this? If not in lexer.coffee, then where?

As far as I can gather from the various issues threads that discuss adding module support, the planned implementation is to match or almost match the ES2015 syntax for import and export, since it is so Coffee-like already. Then CoffeeScript would output ES2015 code for these lines, while the rest of the CoffeeScript-generated code is unchanged. If destructuring is used as part of import, it would be passed through into the output (since any runtime or transpiler that understands import will also understand destructuring). We would have to add a warning in the documentation that if import or export are used, the developer assumes the responsibility of adding Babel or some other tool to their build pipeline to resolve these import and export statements if the code is to be run in a browser or Node until Node gets native ES2015 module support.

Because this is the plan, it really does seem like almost a “pass-through,” like a JavaScript literal, but we need to do some processing to pull out the symbols so that we don’t redefine them with var. Any suggestions on how that can be achieved? I’m also not sure how to handle the case of import 'some-local-file.js' where we don’t get a symbol to add to our list. Also I suppose we should handle import _ from "#{someFolder}/underscore", which would mean this entire token can’t be a pass-through. Ditto with export default (param) ->—how would you handle this?

With regard to some of your caveats, I feel like a more careful regex could detect broken strings or multiline imports. I’m not sure it should be CoffeeScript’s responsibility to complain about import in an invalid place; why not just let JavaScript worry about that?

I think this PR’s tests are the best we can do until the Node runtime supports modules, unless we want to bring in Babel and get much more complicated. I think I would be okay with string comparison tests for the time being until Node catches up.

[connec]

On MDN it describes import 'module-name' as "Import an entire module for side effects only, without importing any bindings", so we don't need any symbols in that case.

[kieran]

@lydell's example above is still an issue, though no more than any other passthrough code would be currently.

Agreed that a smarter regex might be able to satisfy his last example:

import {
    one,
    two,
    three,
} from 'utils'

but given the wide range of examples in the test file, I think he's correct in wanting to fully parse the statements. One of the many benefits of CS is that it virtually always creates linted JS (backticks excepted).

Partially related: It appears that import statements are kind-of hoisted (and resolved) at compile time SO here

[lydell]

@GeoffreyBooth

1. lexer.coffee needs to be adjusted slightly to output the tokens we need.
2. grammar.coffee needs to be adjusted to accept the new export and import grammar. This is where I expect most code and time to be spent.
3. nodes.coffee needs to be adjusted to output JavaScript code for the export and import grammar parsed in grammar.coffee.
4. Edge-cases need to be worked out, and tests need to be written.

(rewriter.coffee might need adjustments as well, but I don't think so.)

If destructuring is used as part of import

It's not. It's just similar syntax.

import _ from "#{someFolder}/underscore"

We can't do that, because ES2015 can't do that by design.

export default (param) ->

Why is that a problem?

[GeoffreyBooth]

@lydell, I’ve tried taking a crack at this: https://github.com/GeoffreyBooth/coffeescript/commits/import-export

I don’t seem to be getting very far. I’ve written a test that fails, so there’s that at least. I’ve removed import, export and default from the reserved keywords list. And I’ve followed the example of class to try to create a token for import that calls a new Import class, but nothing I seem to do calls the new functions I’ve created.

Do you mind taking a look at the commits on that branch and providing a bit of guidance? I feel like if I can figure out how to hook into CoffeeScript’s stream, I can figure out the rest of how to parse the multitude of import syntaxes etc. I just don’t quite understand how lexer.coffee, grammer.coffee and nodes.coffee fit together. I’m also happy to take this conversation elsewhere if there’s a better forum (or email).

[lydell]

@GeoffreyBooth Your branch seems to be working for me:

$ cake build && cake build:parser && cake test
passed 659 tests in 2.52 seconds 

$ ./bin/coffee -ne 'import a'
Block
  Import
    Value IdentifierLiteral: a

[GeoffreyBooth]

Hmm, I was using cake build:full which I guess wasn’t doing what I thought it was doing:

Thanks for cake build && cake build:parser && cake test, that seems to have done the trick! I’ll try to keep going from here. Any guidance is most appreciated, I’m having a hell of a time deciphering what’s going on in this code.

[lydell]

@GeoffreyBooth It's all about time. Rebuilding the parser is the slowest and most uncommon operation, so that only happens if you run cake build:parser explicitly.

• Changed only test → run cake test
• Changed anything but grammar.coffee → run cake build followed by cake test or ./bind/coffee -bpe 'some code' or whatever.
• Changed grammar.coffee → run cake build && cake build:parser followed by cake test or ./bind/coffee -bpe 'some code' or whatever.
• Before submitting a PR: run cake build:full as a sanity-check.

Also be sure to read through https://github.com/jashkenas/coffeescript/wiki/%5BHowto%5D%20Hacking%20on%20the%20CoffeeScript%20Compiler.

[GeoffreyBooth]

@lydell, thanks, yes I’ve been poring over that page. I wish it mentioned that build:full didn’t include building the parser! I’ve settled on git checkout lib/* && cake build && cake build:parser && cake test for now to be thorough, and automated it with this.

This attempt at adding module support has also attracted @JimPanic from this thread. We’ve been poring over lexer.coffee, grammar.coffee and nodes.coffee. From what I can tell, lexer.coffee’s identifierToken does herculean work—it creates tokens for everything from class foo extends bar to for loops and if statements and just about anything that uses a keyword.

I’ve been torn between the approach in #4160, of creating a new function here e.g. moduleToken that takes higher precedence than identifierToken and gobbles up entire import/export lines with a regex like /^(import|export)\s+[^#\\\n]*/; or in trying to adapt identifierToken to somehow parse a line like import { member1 , member2 as alias2 , [...] } from "module-name". I get the sense that the recommended approach would be to extend identifierToken . . . would you agree? If so, do you mind explaining a little how identifierToken works and how we should extend it?

[lydell]

lexer.coffee splits a string of CoffeeScript code into an array of tokens. A token is what a syntax highlighter would give a certain color to. Each token also has a type. lexer.coffee doesn't care much in which order tokens are. For example, it would split } * && bar into ['}', '*', '&&', 'bar'] even though that string is gibberish.

rewriter.coffee hacks the tokens: It adds a few fake once here and there and such.

grammar.coffee is used to describe a Jison grammar to parse the tokens. Jison generates a parser (lib/coffee-script/parser.js) from that grammar. Array of tokens in, tree of objects (an AST) out. The objects – nodes – are described in nodes.coffee.

nodes.coffee describes all possible nodes in the CoffeeScript "AST". They are also responsible for turning themselves into a string of JavaScript.

I’ve been torn between the approach in #4160, of creating a new function here e.g. moduleToken that takes higher precedence than identifierToken and gobbles up entire import/export lines with a regex like /^(import|export)\s+[^#\\\n]*/

That's absolutely the wrong way to go.

or in trying to adapt identifierToken to somehow parse a line like import { member1 , member2 as alias2 , [...] } from "module-name".

Don't try to parse anything in lexer.coffee. That's not its job. Parsing is done in parser.js, which is generated from the grammar in grammar.coffee.

I get the sense that the recommended approach would be to extend identifierToken . . . would you agree? If so, do you mind explaining a little how identifierToken works and how we should extend it?

Make the identifierToken function output IMPORT and EXPORT tokens for import and export, and probably IMPORT_FROM or something for the from. That should be it for the lexer. Then define the grammar in grammar.coffee. Define dummy nodes in nodes.coffee. When you get the parsing right, make those dummy nodes output valid JavaScript.

[JimPanic]

@lydell: Thank you for your insights! I tried to go about as you said, but added a seperate function instead (that I don't call yet, d'oh). I also defined Import as a statement in the grammar. I'm having a bit of trouble generating the JS code for Obj that I get passed to the Import node from Assignable in the grammar, but I'll get to the bottom of that as well.

Is this the correct direction so far? Any further hints you can provide maybe?

https://github.com/GeoffreyBooth/coffeescript/pull/2/files

[lydell]

@GeoffreyBooth your import-export branch is starting to look really good. Keep up the good work! Also credits to @JimPanic. ✨

[GeoffreyBooth]

@lydell thanks for the encouragement 😃

Regarding your comments above about tracking scope, I did some testing and it seems like we might actually not have to track scope at all. I know this seems to good to be true, so I want to run this by you and maybe you can point out what I’m missing.

So I put the following code into the Babel REPL (see here):

import { foo } from 'lib';
foo = true;

And it returns an error:

repl: "foo" is read-only
  1 | import { foo } from 'lib';
> 2 | foo = true;
    | ^

And this is when I had an “aha” moment. If imported members are always read-only, and it would make sense that the spec would define them as such, then CoffeeScript will never create a var line for them. It would be just as if you were accessing a global object defined outside of the current scope. For example, if you had a one-line CoffeeScript file of window.location.href = 'about:blank', that compiles to window.location.href = 'about:blank';—CoffeeScript doesn’t throw a var window; in there.

Currently in my import-export branch, this CoffeeScript:

import { foo } from 'lib'
foo.bar()

compiles, without me doing anything to track identifiers, into this JavaScript:

import { foo } from 'lib';

foo.bar();

which makes complete sense, since this Coffee:

foo.bar()

compiles into this:

foo.bar();

because CoffeeScript doesn’t create a var foo line anywhere unless you assign something to foo, which you can’t do per the spec. So . . . I don’t need to do anything to track identifiers of imported members?

Going back to the original example, written as Coffee:

import { foo } from 'lib';
foo = true;

compiles to this JavaScript:

var foo;

import { foo } from 'lib';

foo = true;

which of course would trigger an error in Babel. But I’m thinking, maybe that’s okay. This is clearly a syntax error—does it matter whether the error comes from CoffeeScript, Babel, or eventually from a Node/JS runtime that supports import? It’s likewise a syntax error to not put all import and export declarations at the top level, but we could let downstream tools catch that error too. I’m sure there are innumerable potential syntax errors within the import syntax as well; for example, import myDefault, { foo, bar }, baz from 'lib'; throws an error in the Babel REPL too.

So I guess the question is, assuming correct import/export syntax is output correctly, is it okay to allow a downstream compiler or runtime to catch invalid import/export syntax?

[lydell]

I had no idea that the following is illegal JS:

import { foo } from 'lib';
foo = true;

Thanks for letting me know! That definitely changes everything about my scope tracking comment.

I'd say leave everything the way you have it now for import and focus on export instead.

---

I think that the output of CoffeeScript is always valid JavaScript (except invalid regex literals). If so, that is no longer true. So it might be worth looking into finding invalid assignments due to imports in the future. But I'd say don't waste time on it now. It's just a detail.

[michaelficarra]

@lydell That's not a syntax error. It's only a syntax error if there's duplicate declarations (let/var, let/let, let/const, import/var, etc). The only allowed duplicates are old style bindings: var/var, var/FD, var/parameter, etc.

[lydell]

@michaelficarra Is it a runtime error then, or not an error at all (except in babel)?

[GeoffreyBooth]

@michaelficarra would also love your input on coffeescript6/discuss#21 if you get a chance. I asked over there if anyone could comment on how good (or not) the CoffeeScript Redux project would be as a starting point for making a new CoffeeScript compiler that output ES2015+.

[GeoffreyBooth]

@lydell, I noticed while testing my branch that compiling a full file, for example:

import { hello } from './hello.js'

hello.sayHi()

produces:

(function() {
  import { hello } from './hello.js';

  hello.sayHi();

}).call(this);

which Babel chokes on:

{ SyntaxError: unknown: 'import' and 'export' may only appear at the top level (2:2)
  1 | (function() {
> 2 |   import { hello } from './hello.js';
    |   ^

however if the CoffeeScript compiler is given the flag bare: true, this “top-level function safety wrapper” isn’t output, and Babel is fine.

How should we handle this? If a file contains import or export, bare is automatically set to true? Should the import and export statements be hoisted above the safety wrapper? (That might be tricky with an export that’s a large block function.) Or just say in the docs that when using modules, set bare: true?

[lydell]

@GeoffreyBooth In my opinion, bare should be automatically set to true if a file contains import or export. There's no need for a safety wrapper in module code.

[michaelficarra]

@lydell Yeah, it would be a runtime error like const a = 0; a = 1;. Babel shouldn't reject these programs. That is the responsibility of a linting tool. Babel even rejects things like const a = 0; if (false) a = 1;.

[wspringer]

I'm trying to work out what it means that the issue has been closed, not accepted, and the discussion is still continuing. Is there any chance some of this will ever get accepted in CoffeeScript?

[GeoffreyBooth]

The original code submitted in this PR won’t be accepted, but we’re working on new code here. When it’s ready, I’ll submit it as a new pull request. Apologies for the confusion caused by us hijacking an old PR thread for discussion; the new PR was inspired by this old one.