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

CoffeeScript files are bare #1125

Closed
TrevorBurnham opened this Issue May 18, 2011 · 16 comments

Comments

Projects
None yet
8 participants
@TrevorBurnham

TrevorBurnham commented May 18, 2011

I noticed that in Rails 3.1 beta 1, you can do things like this:

# A.coffee
foo = 'bar'

# B.coffee
alert foo

That's kind of scary. The way CoffeeScript does scoping assumes that different files are isolated from one another, with specific resources shared by exporting to window (or other global objects). My first thought was that Sprockets was concatenating the .coffee files before compiling them, but no, there's no wrapper at all: foo is in the global scope.

I'm having a hard time tracking down whether this is a Rails issue, a Sprockets issue, or a Tilt issue; the ruby-coffee-script gem definitely has bare = false as its default. Where is it getting overridden, and why?

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim May 18, 2011

Contributor

@josh, @dhh, are we concatenating before compiling it to JS? Seems to be more sane to compile then concat?

Contributor

josevalim commented May 18, 2011

@josh, @dhh, are we concatenating before compiling it to JS? Seems to be more sane to compile then concat?

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh May 18, 2011

Member

Confirmed this as a bug. We'll get it fixed.

Member

dhh commented May 18, 2011

Confirmed this as a bug. We'll get it fixed.

@ghost ghost assigned josh May 18, 2011

@josevalim

This comment has been minimized.

Show comment
Hide comment
@josevalim

josevalim May 18, 2011

Contributor

Assigned to milestone 3.1.

Contributor

josevalim commented May 18, 2011

Assigned to milestone 3.1.

@TrevorBurnham

This comment has been minimized.

Show comment
Hide comment
@TrevorBurnham

TrevorBurnham May 18, 2011

Awesome. Won't have to warn folks about this in my talk in 2 minutes, then. :)

On May 18, 2011, at 11:15 AM, josevalimreply@reply.github.com wrote:

Assigned to milestone 3.1.

Reply to this email directly or view it on GitHub:
#1125 (comment)

TrevorBurnham commented May 18, 2011

Awesome. Won't have to warn folks about this in my talk in 2 minutes, then. :)

On May 18, 2011, at 11:15 AM, josevalimreply@reply.github.com wrote:

Assigned to milestone 3.1.

Reply to this email directly or view it on GitHub:
#1125 (comment)

@josh

This comment has been minimized.

Show comment
Hide comment
@josh
Member

josh commented May 19, 2011

@josh josh closed this May 19, 2011

@house9

This comment has been minimized.

Show comment
Hide comment
@house9

house9 May 26, 2011

might be nice to have an option to force true, something like

//= require initialize, :bare => true
//= require jquery
//...

house9 commented May 26, 2011

might be nice to have an option to force true, something like

//= require initialize, :bare => true
//= require jquery
//...

@TrevorBurnham

This comment has been minimized.

Show comment
Hide comment
@TrevorBurnham

TrevorBurnham May 26, 2011

Hmm, I see a problem with that approach: If one file requires initialize with bare enabled, and another requires initialize with bare disabled... what happens?

To be clear: The only use case for bare is for users who fully understand CoffeeScript scoping to shave off a few bytes. And even then, the optimization is so small (less than a dozen minified bytes per file) that it's hard to see it as significant. Users who really care that much about byte-shaving would want to hand-optimize the JS generated by CoffeeScript anyway.

FWIW, here's how to enable bare globally (not recommended): http://stackoverflow.com/questions/6099342/how-can-i-use-option-bare-in-rails-3-1-for-coffeescript/6099872#6099872

TrevorBurnham commented May 26, 2011

Hmm, I see a problem with that approach: If one file requires initialize with bare enabled, and another requires initialize with bare disabled... what happens?

To be clear: The only use case for bare is for users who fully understand CoffeeScript scoping to shave off a few bytes. And even then, the optimization is so small (less than a dozen minified bytes per file) that it's hard to see it as significant. Users who really care that much about byte-shaving would want to hand-optimize the JS generated by CoffeeScript anyway.

FWIW, here's how to enable bare globally (not recommended): http://stackoverflow.com/questions/6099342/how-can-i-use-option-bare-in-rails-3-1-for-coffeescript/6099872#6099872

@house9

This comment has been minimized.

Show comment
Hide comment
@house9

house9 May 26, 2011

ok, that makes sense, thanks for the info on globally setting bare
don't want to pollute the issues section with my questions, but if you have time can you check out the question I just posted on stack overflow: http://stackoverflow.com/questions/6133235/rails-3-1-rc1-javascript-and-asset-pipeline

Thanks

house9 commented May 26, 2011

ok, that makes sense, thanks for the info on globally setting bare
don't want to pollute the issues section with my questions, but if you have time can you check out the question I just posted on stack overflow: http://stackoverflow.com/questions/6133235/rails-3-1-rc1-javascript-and-asset-pipeline

Thanks

@maxhawkins

This comment has been minimized.

Show comment
Hide comment
@maxhawkins

maxhawkins Jun 8, 2011

If you have coffeescript classes in separate files that reference each other it would be nice to have them all live in a single closure. Otherwise the classes available in the scope of the class declaration depend on the order in which you required the files.

Maybe there could be a #= namespace option that would put files into the same closure?

maxhawkins commented Jun 8, 2011

If you have coffeescript classes in separate files that reference each other it would be nice to have them all live in a single closure. Otherwise the classes available in the scope of the class declaration depend on the order in which you required the files.

Maybe there could be a #= namespace option that would put files into the same closure?

@TrevorBurnham

This comment has been minimized.

Show comment
Hide comment
@TrevorBurnham

TrevorBurnham Jun 8, 2011

Max, I'm not sure what you mean by "otherwise the classes available in the scope of the class declaration depend on the order in which you required the files"... even if they're declared in the same file, the order still matters:

class A
  console.log B # undefined

class B

TrevorBurnham commented Jun 8, 2011

Max, I'm not sure what you mean by "otherwise the classes available in the scope of the class declaration depend on the order in which you required the files"... even if they're declared in the same file, the order still matters:

class A
  console.log B # undefined

class B
@josh

This comment has been minimized.

Show comment
Hide comment
@josh

josh Jun 8, 2011

Member

@maxhawkins

If you have coffeescript classes in separate files that reference each other it would be nice to have them all live in a single closure.

window.Foo = class Foo

Otherwise the classes available in the scope of the class declaration depend on the order in which you required the files.

Hence #= require

#= require foo
window.Bar = Bar extends Foo
Member

josh commented Jun 8, 2011

@maxhawkins

If you have coffeescript classes in separate files that reference each other it would be nice to have them all live in a single closure.

window.Foo = class Foo

Otherwise the classes available in the scope of the class declaration depend on the order in which you required the files.

Hence #= require

#= require foo
window.Bar = Bar extends Foo
@maxhawkins

This comment has been minimized.

Show comment
Hide comment
@maxhawkins

maxhawkins Jun 8, 2011

Ah, you're both right. I had a typo in my code.

Sorry to pollute this space.

maxhawkins commented Jun 8, 2011

Ah, you're both right. I had a typo in my code.

Sorry to pollute this space.

@dukejones

This comment has been minimized.

Show comment
Hide comment
@dukejones

dukejones Sep 22, 2011

Defaulting to wrapping every file in its own closure is the single worst default I have ever seen in a language. When I am programming in Ruby, and I write "class Foo", I expect to be able to access Foo everywhere. That is what makes it useful. However, with a closure around my code, in Coffeescript I have to write "class window.Foo". It's ugly, it's non-semantic, it is an obnoxious mental speed bump. Or, after the definition, window.Foo = Foo. Now I'm repeating myself.

I have not seen a javascript library worth anything that pollutes the global namespace with what should be private variables. Anyone who knows javascript and wants to keep their code tight, just wraps their code in a closure. (And usually setting a top-level object to what the closure returns.)

What this default is doing is essentially pandering to neophytes who do not know javascript, and forcing everyone else to write non-semantic or repetitive code. I expect better from something as awesome as Coffeescript.

dukejones commented Sep 22, 2011

Defaulting to wrapping every file in its own closure is the single worst default I have ever seen in a language. When I am programming in Ruby, and I write "class Foo", I expect to be able to access Foo everywhere. That is what makes it useful. However, with a closure around my code, in Coffeescript I have to write "class window.Foo". It's ugly, it's non-semantic, it is an obnoxious mental speed bump. Or, after the definition, window.Foo = Foo. Now I'm repeating myself.

I have not seen a javascript library worth anything that pollutes the global namespace with what should be private variables. Anyone who knows javascript and wants to keep their code tight, just wraps their code in a closure. (And usually setting a top-level object to what the closure returns.)

What this default is doing is essentially pandering to neophytes who do not know javascript, and forcing everyone else to write non-semantic or repetitive code. I expect better from something as awesome as Coffeescript.

@Mike-Petersen

This comment has been minimized.

Show comment
Hide comment
@Mike-Petersen

Mike-Petersen Oct 10, 2011

Is there an easy way around this? I want to split my code into seperate files, but with this behaviour i need to add every class that has its own file to window.

Hope there is a simpler solution.

Mike-Petersen commented Oct 10, 2011

Is there an easy way around this? I want to split my code into seperate files, but with this behaviour i need to add every class that has its own file to window.

Hope there is a simpler solution.

@TrevorBurnham

This comment has been minimized.

Show comment
Hide comment
@TrevorBurnham

TrevorBurnham Oct 12, 2011

@OdaniaIT The easy way around this is... to add every class that has its own file to window. Seriously, this is the norm in JavaScript—look at the jQuery source, for instance. Every file is wrapped in a closure so that it can have its own local variables. CoffeeScript just automates this ubiquitous pattern.

Here's the good news: this (i.e. @) points to window from that closure, which means that instead of

class window.Foo

you can just write

class @Foo

Once you've defined it, you can just refer to the class as Foo from anywhere in your application.

TrevorBurnham commented Oct 12, 2011

@OdaniaIT The easy way around this is... to add every class that has its own file to window. Seriously, this is the norm in JavaScript—look at the jQuery source, for instance. Every file is wrapped in a closure so that it can have its own local variables. CoffeeScript just automates this ubiquitous pattern.

Here's the good news: this (i.e. @) points to window from that closure, which means that instead of

class window.Foo

you can just write

class @Foo

Once you've defined it, you can just refer to the class as Foo from anywhere in your application.

@Mike-Petersen

This comment has been minimized.

Show comment
Hide comment
@Mike-Petersen

Mike-Petersen Oct 13, 2011

Thanks for the reply. I didn't know that i can write @foo, makes it a bit simpler.

Mike-Petersen commented Oct 13, 2011

Thanks for the reply. I didn't know that i can write @foo, makes it a bit simpler.

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