Proposal: Generate wrapper only when needed

[TrevorBurnham]

Lately, Google Dart has been getting some well-deserved flack for a trivial "Hello, world" example generating thousands of lines of JS output. Meanwhile, though, CoffeeScript is generating 3 lines when it could be generating just 1. Since

 console.log 'Hello, world!'

contains no scoped variables, I don't see any benefit to generating the output

(function() {
  console.log('Hello, world!');
}).call(this);

instead of just

console.log('Hello, world!');

I propose that the wrapper only be added when it has an effect on scope. What do other folks think?

[davidchambers]

I would love to see this. A common pattern is...

jQuery ($) ->
  # rest of file resides inside this function

When manually compiling a single file it's easy enough to use --bare. When writing a build script, though, or working on a team with people less familiar with CoffeeScript, compiling files differently based on their contents is problematic. On my team we've abandoned --bare to minimize friction. I'd love to see the top-level function wrapper omitted in cases where it's unnecessary (irrespective of --bare).

+1

[jashkenas]

I agree -- this would be rad.

[michaelficarra]

I love it. +1.

[paulmillr]

👍, great idea.

[satyr]

Watch also for top level returns which were guaranteed to work without --bare.

[michaelficarra]

@satyr: See #1319. I'd prefer they're disallowed entirely. A top-level return makes no sense, even if it is useful for control flow.

[showell]

FYI there will be a small doc fix needed, as it would no longer true that "all CoffeeScript output is wrapped in an anonymous function: (function(){ ... })();".

I'm in the minority here, but I don't understand why this fix is unnecessary. It seems to fix a non-problem. Google Dart is getting flack for a 17,000-line hello-world. CS has a perfectly reasonable 3-line hello-world that's probably already documented several places out in the wild. The safety wrapper is needed in 95% of cases, and in the 5% of cases where it could be optimized away, it's almost always in example code, and it's never an actual bottleneck.

The complexity cost is pretty small here, so I'm -0.001.

[showell]

P.S. While the safety wrapper seems like a misfeature for hello-world, it makes it crystal clear that the author intended the code to be safe. I'm not sure if humans care, but it's possible that asset management systems use this as a cue. It's probably moot in most cases.

[showell]

FYI Github uses the safety wrapper as a cue to suppress .JS files in diffs.

https://github.com/github/linguist/blob/master/lib/linguist/blob_helper.rb

    # Internal: Is the blob JS generated by CoffeeScript?
    #
    # Requires Blob#data
    #
    # CoffeScript is meant to output JS that would be difficult to
    # tell if it was generated or not. Look for a number of patterns
    # outputed by the CS compiler.
    #
    # Return true or false
    def generated_coffeescript?
      return unless extname == '.js'

      if lines[0] == '(function() {' &&     # First line is module closure opening
          lines[-2] == '}).call(this);' &&  # Second to last line closes module closure
          lines[-1] == ''                   # Last line is blank

        score = 0

        lines.each do |line|
          if line =~ /var /
            # Underscored temp vars are likely to be Coffee
            score += 1 * line.gsub(/(_fn|_i|_len|_ref|_results)/).count

            # bind and extend functions are very Coffee specific
            score += 3 * line.gsub(/(__bind|__extends|__hasProp|__indexOf|__slice)/).count
          end
        end

        # Require a score of 3. This is fairly arbitrary. Consider
        # tweaking later.
        score >= 3
      else
        false
      end
    end

[yuchi]

Good catch! Probably that section of the blob_helper should be modified to simply add some big score if the safety wrapper is found, but it should not require it anymore.

[showell]

@yuchi Even with modifications, the blob_helper code would have trouble distinguishing whether small JS files were hand-written or CS-generated, once the safety wrapper was removed. This is just a nuisance, of course, but the safety wrapper itself is nothing more than a nuisance as well.

[jashkenas]

Ping @josh, so he's aware of this potential change.

[josh]

@showell Right, these simple cases with or without the wrapper have always been too hard to detect. I'm not really worried about it. But if you have any improvements, please submit a pull. That's why it's open source ;)

Overall this change is pretty neat.