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

Add header with version number to compiled JS #1778

Closed
TrevorBurnham opened this issue Oct 18, 2011 · 13 comments
Closed

Add header with version number to compiled JS #1778

TrevorBurnham opened this issue Oct 18, 2011 · 13 comments

Comments

@TrevorBurnham
Copy link
Collaborator

It would be nice if .js files compiled from CoffeeScript started with a simple comment like

// Generated by CoffeeScript 1.1.2

There would be three major advantages to this:

  1. Documentation. The implicit message would be, "Do not modify this JS file directly."
  2. Compiled file detection. It'd be a big help for projects like GitHub that have good reason to treat compiled JS files differently from hand-coded JS. We may have just broken their current algorithm with Proposal: Generate wrapper only when needed #1774...
  3. Maintainability. If a project is dormant for several months, there's a chance that the project's CoffeeScript code will no longer work as intended with the latest version of CoffeeScript. Knowing exactly which version of CoffeeScript was used for the project before would make migration a lot easier.

That last reason is, in my view, the big one. It's great that CoffeeScript has been stable for the last several months (hurray, my book isn't obsolete yet!), but it'd be nice to make some breaking changes in the future, and most CoffeeScript projects aren't documenting the version of CoffeeScript they're being compiled with.

As to the precise implementation: CoffeeScript.compile should accept a header flag, which would be set to true when the compiler is invoked from command.coffee to generate a JS file. That way, it wouldn't show up in cases like coffee -pe "foo()" or in environments like Try CoffeeScript. Sound good?

@jashkenas
Copy link
Owner

As @josh mentions in the other thread -- we've discussed this before ... but I think that Trevor's approach here is very nice and non-invasive. You won't see it littering up small examples too much.

But. What about folks who are piping into files?

coffee -p script.coffee > script.js.erb

... or whatever.

@MichaelBlume
Copy link

+1

@TrevorBurnham
Copy link
Collaborator Author

But. What about folks who are piping into files?

I think it's acceptable for the only options with the command-line coffee to be either "generate a file, get a header" or "don't generate a file, don't get a header." I don't want to add yet another flag to poor little coffee... and I don't imagine you do either?

@jashkenas
Copy link
Owner

and I don't imagine you do either?

You've got that right.

@geraldalewis
Copy link
Contributor

I really like this idea, for maintainability in particular.

@misfo
Copy link

misfo commented Oct 18, 2011

What about javascript files that are built with CoffeeScript compilation as just part of a larger build process? For example, would you then have the build process for extras/coffee-script.js put this comment at the top of the file manually?

@TrevorBurnham
Copy link
Collaborator Author

@misfo The option would be available as a flag, just as bare is:

CoffeeScript.compile coffee, {bare: false, header: true}

@misfo
Copy link

misfo commented Oct 18, 2011

@TrevorBurnham That makes sense. But this won't help github recognize that extras/coffee-script.js was compiled (mostly) from CoffeeScript, since uglifiy will have removed any of those comments. So will that same comment get added back into the built JS file by the build process?

@michaelficarra
Copy link
Collaborator

@misfo: Minified JS is recognised separately from generated JS. I believe they recognise the fact that the average line length is very long. So there's no need for an identifying comment in minified output.

@josh
Copy link
Contributor

josh commented Oct 18, 2011

@showell
Copy link

showell commented Oct 18, 2011

+1 Not commenting on all the details of the thread, but +1 on the concept.

@TrevorBurnham
Copy link
Collaborator Author

Pull request is at #1793. Have at it.

By the way, while testing this I noticed that on the current master, every \n in compiled JS is being turned into a \n\n [Edit: Only at the top level], so that for instance

x = 5
console.log x

gets compiled to

(function() {
  var x;

  x = 5;

  console.log(x);

}).call(this);

Note sure what caused this, but I'm assuming it wasn't intentional... [Edit: Ah, apparently it was. The relevant commit is d2b0404, and there was some discussion at #1713.]

@jashkenas
Copy link
Owner

Closing this ticket in favor of continuing the conversation over on the pull request ...

(aside: Shouldn't there be some way of creating a pull request that joins an existing ticket? Most of our conversations end up splitting into two or more parts because of pull requests.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants