Modules documentation #4309

GeoffreyBooth · 2016-09-15T07:21:13Z

First draft. Relies on #4308.

GeoffreyBooth · 2016-09-15T07:25:41Z

For whatever reason, the JavaScript syntax highlighting doesn’t treat import and export as keywords:

lydell · 2016-09-15T15:25:10Z

Looks nice!

Did we discuss somewhere else something about documenting --bare+import/export stuff?

vendethiel · 2016-09-15T15:35:23Z

documentation/index.html.js

+    <p>
+      Note that the CoffeeScript compiler <strong>does not resolve modules</strong>; writing an
+      <code>import</code> or <code>export</code> statement in CoffeeScript will produce an
+      <code>import</code> or <code>export</code> statement in the resulting output JavaScript.


resulting output JavaScript? I'd propose "resulting JavaScript" or "resulting output" or "JavaScript output" or...

vendethiel · 2016-09-15T15:38:56Z

src/lexer.coffee

@@ -115,7 +115,11 @@ exports.Lexer = class Lexer
    if id is 'from' and @tag() is 'YIELD'
      @token 'FROM', id
      return id.length
-    if id is 'as' and (@seenImport or @seenExport) and @tag() in ['IDENTIFIER', 'IMPORT_ALL', 'EXPORT_ALL']
+    if id is 'as' and @seenImport and (@tag() is 'IDENTIFIER' or @value() is '*')


not sure why the commit from #4308 is here...

It was necessary to get my example code to compile.

I suppose it can be rebased out now.

vendethiel · 2016-09-15T15:39:22Z

@lydell we just said it'd be nice to document it, IIRC.

GeoffreyBooth · 2016-09-16T03:03:53Z

@lydell, good catch. Updated.

I wonder if we really need to say anything else? I like the rather minimal documentation that lets the example speak for itself. Or should I link to something explaining what modules are, or to the MDN import or export pages, or anything like that?

…adds our new reserved words

GeoffreyBooth · 2016-09-16T03:28:02Z

Cakefile

@@ -54,6 +54,10 @@ codeFor = ->
    js = js.replace /^\/\/ generated.*?\n/i, ''

    cshtml = "<pre><code>#{hljs.highlight('coffeescript', cs).value}</code></pre>"
+    # Temporary fix until highlight.js adds support for newer CoffeeScript reserved words


@lydell how do you feel about this? How important is clear highlighting? I feel like we don’t want to wait until highlight.js updates its definition for CoffeeScript, and then we update the version of highlight.js we’re using.

I think it's fine to special-case here. However, I think we should use the power of regex instead of a for loop ;)

cshtml = cshtml.replace /import|export|from|as|default/g, '$& '

They’re actually not the same. We need to search for the space after the keyword, or else as matches within class=". But if we replace including the space, we get as . Maybe this could be done with an even cleverer regex, but I’m not sure it’s worth the effort.

Ah, sorry, I didn’t see that space! This should do the trick:

cshtml = cshtml.replace /(import|export|from|as|default) /g, '$1 '

lydell · 2016-09-16T05:11:04Z

bower.json

@@ -14,7 +14,7 @@
  "devDependencies": {
    "uglify-js": "~2.2",
    "jison": ">=0.2.0",
-    "highlight.js": "~8.0.0",
+    "highlight.js": "~9.6.0",


I'm confused. #3981 (comment):

Is there a reason we have this bower.json? It doesn’t seem to be used.

The bower.json is (was) for people using Bower as part of their compilation steps. Not sure it's needed anymore.

lydell · 2016-09-16T05:16:21Z

documentation/index.html.js

+      be output without a <a href="#lexical-scope">top-level function safety wrapper</a>;
+      in other words, importing or exporting modules will automatically trigger
+      <a href="#usage">bare</a> mode for that file. This is because per the ES2015 spec,
+      <code>import</code> or <code>export</code> statements must occur at the topmost scope.


I like that you link to other parts of the documentation. Here's a quote from the #lexical-scope section:

If you'd like to create top-level variables for other scripts to use, attach them as properties on window, or on the exports object in CommonJS. The existential operator (covered below), gives you a reliable way to figure out where to add them; if you're targeting both CommonJS and the browser: exports ? this

That mentions even CommonJS – should it perhaps mention import/export as well?

Also, I'm having a hard time deciding where this --bare stuff fits best – in the #modules section, the #lexical-scope section, the #usage section (does the coffee --help output need to be updated as well?), all of them … ? What's your opinion?

--help seems fine to me.

I don’t understand exports ? this. Is that even CoffeeScript? Shouldn’t it be something like exports ?= this?

I think this is what is hinted as:

attach them as properties on window, or on the exports object

# Browser globals only: window.myExport = myExport # CommonJS only: exports.myExport = myExport # Support either: global = exports ? window global.myExport = myExport # `this` at the top level is the global object. Theoretically support more stuff: global = exports ? this global.myExport = myExport

Not sure if we should go down this sub-quest of improving that paragraph of documentation “while at it,” or just leave it be and be happy with the addition of mentioning export statements.

There’s no convenient way to support all three. You either have to know you’re using modules or you’re targeting the browser/CommonJS. You can’t even do some kind of check as far as I’m aware, like “if export is supported...”. I guess just leave it as I’ve amended it, with the mention of export?

lydell · 2016-09-16T05:19:20Z

Cakefile

@@ -54,6 +54,10 @@ codeFor = ->
    js = js.replace /^\/\/ generated.*?\n/i, ''

    cshtml = "<pre><code>#{hljs.highlight('coffeescript', cs).value}</code></pre>"
+    # Temporary fix until highlight.js adds support for newer CoffeeScript reserved words


I think it's fine to special-case here. However, I think we should use the power of regex instead of a for loop ;)

cshtml = cshtml.replace /import|export|from|as|default/g, '$& '

lydell · 2016-09-16T05:20:35Z

index.html

 }

-square = <span class="function"><span class="keyword">function</span><span class="params">(x)</span> {</span>
+square = <span class="function"><span class="keyword">function</span>(<span class="params">x</span>) </span>{


Are all these little changing due to you updating highlight.js?

I think this PR will be easier to review if you leave out the compiled files. They will be committed when the 1.11.0 release is done.

lydell · 2016-09-16T06:03:45Z

I wonder if we really need to say anything else? I like the rather minimal documentation that lets the example speak for itself. Or should I link to something explaining what modules are, or to the MDN import or export pages, or anything like that?

I think it's good the way it is. You've matched the tone and style of the rest of the documentation nicely, I think.

This reverts commit 96b868a.

GeoffreyBooth

I think I’ve addressed all the notes so far.

GeoffreyBooth · 2016-09-17T16:50:35Z

Alright @lydell, anything else? I wish I could somehow mark your review as done. I think I might prefer the review style we were using in #4300, since when I would commit changes that addressed line notes, those comments would disappear from the stream. In this mode the comments stick around commit after commit, with no way to mark them as “closed” or addressed, as far as I can tell.

GeoffreyBooth · 2016-09-18T18:56:27Z

Sweet! Is there anything else to do before cutting a release?

I was just noticing that the introduction at the top of the docs says that CoffeeScript “will work in every JavaScript runtime” . . .

lydell · 2016-09-18T19:08:27Z

Sweet! Is there anything else to do before cutting a release?

I don’t think so. Last Friday I started plowing through the commit log since the last release (that’s one year ago!) to make a change log, but I didn’t get very far.

I was just noticing that the introduction at the top of the docs says that CoffeeScript “will work in every JavaScript runtime” . . .

Well, that’s not been true since yield was added …

jashkenas · 2016-09-20T15:22:36Z

Well, that’s not been true since yield was added …

Maybe it should be, though? I kind of like the notion of CoffeeScript 1.X being super-compatible just-works-anywhere — even in bizarre environments like Adobe Illustrator ... and 2.X being the "modern" flavor. Probably too late for that though.

GeoffreyBooth · 2016-09-20T16:50:31Z

A bit late since we added generators. Modules I don’t think anyone would expect to work without further build-chain steps. Maybe we should just add an asterisk after “every JavaScript runtime,” linking to a footnote or tooltip like “(except for certain ES2015+ features, modules and generators, that are opt-in by using the feature)”.

Modules documentation first draft

5b74433

GeoffreyBooth assigned lydell Sep 15, 2016

vendethiel reviewed Sep 15, 2016

View reviewed changes

GeoffreyBooth added 2 commits September 15, 2016 19:53

Merge remote-tracking branch 'upstream/master' into modules-docs

54c189d

Add note about bare output

72f7dba

GeoffreyBooth added 3 commits September 15, 2016 20:25

Update highlight.js

633dcb1

Fix highlighting for CoffeeScript modules syntax, until highlight.js …

c162917

…adds our new reserved words

Updated generated documentation

96b868a

GeoffreyBooth commented Sep 16, 2016

View reviewed changes

lydell requested changes Sep 16, 2016

View reviewed changes

GeoffreyBooth added 2 commits September 16, 2016 12:57

Revert "Updated generated documentation"

50a112d

This reverts commit 96b868a.

Suggest export as an option for exporting symbols to other scripts

502be0a

GeoffreyBooth commented Sep 16, 2016

View reviewed changes

Replace syntax highlighting for loop with regex

1745601

lydell approved these changes Sep 18, 2016

View reviewed changes

lydell merged commit a8b77fb into jashkenas:master Sep 18, 2016

GeoffreyBooth deleted the modules-docs branch September 18, 2016 19:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modules documentation #4309

Modules documentation #4309

GeoffreyBooth commented Sep 15, 2016

GeoffreyBooth commented Sep 15, 2016

lydell commented Sep 15, 2016

vendethiel Sep 15, 2016

vendethiel Sep 15, 2016

GeoffreyBooth Sep 16, 2016

connec Sep 16, 2016

vendethiel commented Sep 15, 2016

GeoffreyBooth commented Sep 16, 2016

GeoffreyBooth Sep 16, 2016

lydell Sep 16, 2016 •

edited

Loading

GeoffreyBooth Sep 16, 2016

lydell Sep 17, 2016

lydell Sep 16, 2016

vendethiel Sep 16, 2016

lydell Sep 16, 2016

vendethiel Sep 16, 2016

GeoffreyBooth Sep 16, 2016

lydell Sep 17, 2016

GeoffreyBooth Sep 17, 2016

lydell Sep 16, 2016 •

edited

Loading

lydell Sep 16, 2016

lydell commented Sep 16, 2016

GeoffreyBooth left a comment

GeoffreyBooth commented Sep 17, 2016

GeoffreyBooth commented Sep 18, 2016

lydell commented Sep 18, 2016

jashkenas commented Sep 20, 2016

GeoffreyBooth commented Sep 20, 2016

Modules documentation #4309

Modules documentation #4309

Conversation

GeoffreyBooth commented Sep 15, 2016

GeoffreyBooth commented Sep 15, 2016

lydell commented Sep 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vendethiel commented Sep 15, 2016

GeoffreyBooth commented Sep 16, 2016

Choose a reason for hiding this comment

lydell Sep 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lydell Sep 16, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lydell commented Sep 16, 2016

GeoffreyBooth left a comment

Choose a reason for hiding this comment

GeoffreyBooth commented Sep 17, 2016

GeoffreyBooth commented Sep 18, 2016

lydell commented Sep 18, 2016

jashkenas commented Sep 20, 2016

GeoffreyBooth commented Sep 20, 2016

lydell Sep 16, 2016 •

edited

Loading

lydell Sep 16, 2016 •

edited

Loading