jashkenas / coffeescript

Unfancy JavaScript
https://coffeescript.org/
MIT License
16.52k stars 1.98k forks source link

compiler option to use ES5 bind method. #1363

Closed krisnye closed 7 years ago

krisnye commented 13 years ago

current output is a __bind method and this:

__bind(function() {}, this)

desired output with a new compiler option set would be this:

function(){}.bind(this)

jashkenas commented 13 years ago

I'm afraid not. CoffeeScript only compiles to JS that runs "everywhere" (meaning even IE). __bind could certainly use Function.prototype.bind if it exists, however.

krisnye commented 13 years ago

True, but I'm just talking about a compiler OPTION. I define Function.prototype.bind for browsers that don't have it anyways, so the generated code is cleaner for me if I can compile with the option to just use it. There is some chance that browsers may have a more efficient native implementation of the function so I like to use the one they provide if at all possible.

erisdev commented 13 years ago

A compiler option would be even worse because it needlessly complicates things. When Function.prototype.bind becomes ubiquitous, this will no doubt find its way into CoffeeScript; for now, it's probably best to leave the compiler output as is and make CoffeeScript's __bind use it if available,

krisnye commented 13 years ago

How about just adding an ES5 compile option? If set then the compiler assumes that all ES5 methods are available in the target environment.

Keep in mind that I'm not just looking into using CoffeeScript on the client. I use Javascript for server side code and for command line build utilities as well. In both of those environments I know precisely what the execution environment will be.

krisnye commented 13 years ago

Been writing my first coffeescript. Liking this alot, good form. Now that I'm seeing more of the code it generates at the top though, I'm really thinking an ES5 compile option would be excellent. I will be individually compiling hundreds of different classes in their own .coffee files and it wouldn't do to have the boilerplate function definitions at the top of every single file. If you don't have time, I will probably be able to fork this and implement the option on my own.

Don't want this at the top of every one of my files. (They will be merged into one big file for release site later, but I will compile them independently.)

var slice = Array.prototype.slice, indexOf = Array.prototype.indexOf || func tion(item) { for (var i = 0, l = this.length; i < l; i++) { if (this[i] === item) return i; } return -1; }, __bind = function(fn, me){ return function(){ return fn.apply(me, arguments); }; };

jashkenas commented 13 years ago

It's not about not having time -- it's about it being a bad idea. Unless you take pains to use ES5 explicitly, all of the code that CoffeeScript generates should be capable of running on any JS runtime.

michaelficarra commented 13 years ago

Yeah, exactly as @erisdiscord says. When ES5 functions become ubiquitous (so, in Jeremy's mind, when IE<9 dies completely), we will look at this, but until then we need to support all widely available JS engines.

edit: So new ticket goal: add Function.prototype.bind || before the function assigned to __bind, correct? I am in favor of this.

krisnye commented 13 years ago

I can see you guys are against this idea. I feel like we're miscommunicating though, so I will take one more stab at it.

I run my javascript in an environment that works in ALL browsers. I include a file which defines compatible methods for all missing ES5 functions. This includes the following:

if (Function.prototype.bind == null) Function.prototype.bind = function(self) { ..... implementation here for non ES5 browsers };

if (Array.prototype.indexOf == null) .... if (Array.prototype.map == null) ... if (Array.prototype.filter == null) ... if (Object.defineProperty == null) ...

etc, etc.

IF a browser does not have ES5 functions yet, then I define compatible methods for them. This is not an uncommon pattern. It allows me to use the new methods before the browsers provide them on all platforms.

Now what I am asking for is a CoffeeScript.compile option that if set, would assume the presence of ES5 functions like bind/indexOf etc on the built in objects, and use them. This would not break any browsers, because only people who knew they already provided the ES5 compatible functions like me would use the option.

When every individual file I compile writes common methods to the top of itself then it is not as clean as it could be for me, and it makes my final generated file larger than it should otherwise be.

@jashkenas- I have no idea what you mean "Unless you take pains to use ES5 explicitly"? In my situation all the relevant ES5 functions are always present on all browsers.

@erisdiscord- When you say "but until then we need to support all widely available JS engines", I agree 100%. I have no idea how an OPTIONAL compile flag changes that situation.

I have no problem with you guys not wanting to implement this. That is completely reasonable and I'm sure you have more pressing concerns. I am just concerned that you both appear to be misunderstanding the situation.

http://code.google.com/p/es5/ is an example library that provides ES5 functions on all browsers. Anyone using a similar library might choose to compile with the ES5 target option and avoid unnecessary repeated declarations of ES5 functions.

erisdev commented 13 years ago

I don't think anybody's against the idea of adding support for ES5; just that it will have to be put off until ES5 is ubiquitous.

I am against an optional compiler flag because it adds complexity to the compiler.

michaelficarra commented 13 years ago

@krisnye: I believe @jashkenas meant "unless you use Object.keys or some other ES5 extension in your code, it will run on all available JS engines".

eirikurn commented 13 years ago

I disagree with the reasoning that CS should always run on all engines. I feel the current view is ignorant and causes bloating in the generated code, especially in large projects. It's making assumptions about the use-cases of the language, which are currently quite limited.

I would be very interested in a poll to see where CS code is running. I would think that the following markets are seeing the biggest rise:

Since these markets have the easiest time integrating CS, compared to typical web-development environments (ASP.NET, PHP, Python). Developers in these markets don't care at all if their code runs on IE, if anything, they love that it won't and explicitly use many ES5 features. Even just the mobile web app market should be reason enough to have this kind of flag, where speed and code size matters most.

Waiting for "ES5 to be ubiquitous" is a fallacy, since IE6-8 will keep a solid slice of browser share for the next 5-10 years thanks to corporations.

Sorry for the trolling, I really love this language and hope it continues to grow. Maybe we just need something different, like Zepto.js is to jQuery.

erisdev commented 13 years ago

@eirikurn I would also like to see CoffeeScript move forward and leave legacy JS engines behind, but the man in charge feels otherwise. You're free, of course, to make your own fork and run it as you see fit. One remark in your comment bothers me, though:

I feel the current view is ignorant and causes bloating in the generated code …

I suppose it's fair to say that it does bloat the generated code somewhat, but compiling to a single output file (frequently recommended even with plain JS to limit the number of HTTP requests, by the way) mitigates this since CoffeeScript's boilerplate code is only output once in this case. When CS starts writing ES5-compiliant code, you can recompile and reap the benefits.

On the other hand, the proposed compiler flag would bloat the compiler; I'm not going to argue about performance and optimisation because I just don't know, but the added complexity makes the code base more difficult to maintain. I'm not even sure if the current compiler architecture could handle something like that gracefully, but feel free to correct me if I'm wrong on that count.

On the third hand, changing now and breaking compatibility with a large portion of the browser market is likely to alienate a number of current users. You and I don't care about Internet Explorer, but most web developers have to. Pulling the rug out from under those folks just seems down-right rude. It would probably hurt CoffeeScript's reputation, too.

krisnye commented 13 years ago

@erisdiscord- You keep misrepresenting the situation when you claim that we are proposing something that would break current users. No one has ever said that you should do anything to break current functionality. The option would default to the current behavior. If the option to target="ES5" was set only then would it use ES5 methods directly.

As for complexity... yes, all changes increase complexity. The only thing important is whether or not the complexity is worth it. ES5 is not going anywhere. It will only get more important. It seems to me that the only changes that will happen will be on the final output layer where the javascript is actually created. I won't know until I get a chance to fork the code and look at it though.

Also, concerning compiling to a single js file: We all agree that a single output js is best for release builds. In a debug environment however, it's best to have lot's of individual files for separate classes. You could have a release build process that merges all coffee files in dependency order and then builds them. The problem is that assumes you are using exclusively .coffee files. In reality you will probably have some files authored in javascript by you or third parties that you want to integrate. You may also have files authored in another language that compiles to javascript. It's much easier to individually compile all files to javascript once, and then merge them all into a single file and run a minify on them. I have experience with this. Our debug version has a couple hundred javascript files... our release version is built and minified down to a single 1 meg javascript file.

I'm sorry if I have not come across as polite before. Coffeescript kicks much ass, and I'm only trying to help it kick even more. I want the code to be as tight as possible for those of us using ES5 libraries. I will keep learning more about the system and will have add a feature request in the future that I hope makes sense.

eirikurn commented 13 years ago

Ah, sorry, I hadn't seen or tried the --join feature of coffee. In my projects I've always had dependencies on vanilla js files and micromanaged my assets by using asset managers and asset build systems. In that case, compiling coffee is just one of the steps that run on individual files before being joined and minified.

After playing around with the --join feature, I wouldn't recommended it at all in its current form. It breaks coffee-scripts lexical scoping and variable safety ideas, since all joined files run in the same scope, which is dangerous:

# a.coffee
a = 10
setInterval (-> console.log a), 0

# b.coffee
a = 20

# combined.js
(function() {
  var a;
  a = 10;
  setTimeout((function() { return console.log(a); }), 0);
  a = 20;
}).call(this);

Hopefully this is a bug and will be fixed. Otherwise it doesn't help.

jannschu commented 13 years ago

Although it is closed: What about putting the __bind function in Function.prototype.bind if this does not exist?

michaelficarra commented 13 years ago

@jannschu: check out the above commit.

mvolkmann commented 13 years ago

I strongly agree with the all the people asking for an -es5 option! I'm working on a project where the generated JavaScript will either run in Node.js or in a browser that supports ES5 features. I don't want the generated code to be bloated and less readable because it avoids using those features.

I think a new issue should be opened for this.

jashkenas commented 13 years ago

Just for the record -- even if we do open a new ticket to discuss this, it will be judiciously wontfix'd.

It's a very strong value of CoffeeScript to generate JavaScript that's capable of running on any JS platform -- your intentions notwithstanding. If you end up writing a neat bit for your browser app that you end up wanting to open-source as a standalone library, it should already be inter-operable.

michaelficarra commented 13 years ago

related: #1408

krisnye commented 13 years ago

I am going to have hundreds of individual coffeescript compiled files. Do I really have to have a bunch of compatibility cruft at the top of every single file?

I am providing ES5 bindings myself so that it will already run in any browser. (Besides the fact that my application by design uses features that will only ever be present in ES5 browsers: Object.defineProperty, WebGL, etc.)

Would you at least consider allowing us to fork and implement this and then consider a pull. We are only asking for a compiler option to target the ES5 platform. This will not affect anyone that doesn't explicitly desire it.

eirikurn commented 13 years ago

I can imagine this being controversial enough to deserve, if not an official option, then a maintained fork like coffee-script-es5. There are just too many projects where the extra cruft will be 100% unwanted and even bad for performance and size.

This is similar to how most choose zepto instead of jquery when developing mobile web-apps, where every byte matters.

jashkenas commented 13 years ago

@krisnye: Ideally, you only have a single copy of the (quite small) compatibility cruft.

But of course, feel free to fork and pull req. -- The CoffeeScript source is annotated with the specific intention that forks and tweaks are easier -- go nuts.

krisnye commented 13 years ago

@jashkenas-

If it was only a single copy of cruft then I wouldn't be pushing as hard. I am authoring a large project in a heterogeneous language environment. Most files are in coffeescript, but some files are in javascript. I cannot merge javascript and coffeescript files together before compiling in order to minimize the cruft. I must convert them all to javascript before calculating dependency order and then merging them.

So... the solution of merging all coffeescript files before releasing will simply not work for a mixed language environment. I am stuck with cruft at the top of every single one of literally hundreds of files.

I'm not really interested in asking you to implement this. You've already done tons of excellent work for free. It would be nice though for you to at least acknowledge that this is a problem. When I get closer to releasing my application, then I will implement this myself.

michaelficarra commented 13 years ago

@mvolkmann: @krisnye: @eirikurn: I'm currently working on https://github.com/michaelficarra/coffee-of-my-dreams in my free time. It may be of interest to you.

krisnye commented 13 years ago

@michaelficarra- I'm watching that project now. Seems pretty ambitious though. I'd recommend making that two separate projects. One project to provide separate compile targets for CoffeeScript and then a separate project that aims to change the language.

Off-topic: I looked at that Coco project as well. Not impressed. I think jashkenas is doing a pretty good job of shepherding the language.

rosenfeld commented 13 years ago

I also agree that this is a very annoying issue in CoffeeScript. I also have multiple CoffeeScript files alongside with other JavaScript files in my browser. I'm also using es5-shim before every page using JS, so it is supported in every browser, including IE. So, having lots of repeated code among my JS generated files like bind and indexOf is not bad only for sending extra bytes over the wire, but because it just seems stupid extra code in environments where we're sure that is ES5 compliant.

I really can't understand why making this an option would be so bad. I didn't read the sources but in my mind it would be something like "printCompatibilityFunctionsIntoHeader() unless options.es5".

Am I missing something?

Please, don't get me wrong. I'm starting to write CS code and I'm loving it (I really hate the JS language), but I still care about my generated JS files... I've just converted lots of JS code to CS and although the CS code is about 30% shorter than the JS code, the generated JS code from CS is about 30% bigger than the original one.

I would like to write cleaner code (which the CS language will allow me) but to avoid wasted code (which this current implementation insists in not doing for this use case).

Could you please elaborate why adding such an option doesn't seem right for you? What are the drawbacks?

michaelficarra commented 13 years ago

@rosenfeld: you're forgetting that those helper functions are also used everywhere throughout the code, not just defined at the top. A quick fix would be to define the helper references assuming that the es5 functions exist (but still changing their signatures sometimes, since they're not all the same as our helpers). But really having multiple compilation targets is just impossible with the current version of the compiler. It needs a rewrite for a few reasons, that being one of the most important.

rosenfeld commented 13 years ago

Ok, now I finally understood that this wouldn't be so trivial. For the case where the signatures differ from ES5, it would be probably better to just change the signature to reflect the specification, don't you think? I'm not sure if that was what you suggested...

That way, a quick fix like what you've suggested would already be a great improvement.

So, did I understand correctly that this could be considered as an issue and that a patch for adding such a "es5" option would be evaluated by this implementation developers? If that's the case, I can try to find some time to read the source and evaluate if I could easily add such an option...

Otherwise, if it's already decided that such option doesn't worth the changes in the compiler, I won't waste my time...

michaelficarra commented 13 years ago

@rosenfeld:

I'm not sure if that was what you suggested

Yes, that was what I was suggesting.

this could be considered as an issue and that a patch for adding such a "es5" option would be evaluated

Pull requests are always welcome.

I can try to find some time to read the source and evaluate if I could easily add such an option...

It would probably take any one of the collaborators less than an hour to fix and test, so it probably wouldn't take you a considerable amount of time. You may want to ask someone on IRC that knows their way around the compiler for some pointers related to your task. Like how the helpers are defined at the bottom of /src/nodes.coffee, the utility function is used to generate references to them and ensure they'll be output, the options are configured in /src/command.coffee, etc.

rosenfeld commented 13 years ago

Thanks for the pointers. I'll try to take a look at it when I find some free time

krisnye commented 13 years ago

It's nice to see this issue moving from "it's a bad idea" to "it's hard to implement". Apparently it will be easier once there is a compiler rewrite. Any idea from the main developers what the timeframe is for a compiler rewrite?

jashkenas commented 13 years ago

I'm afraid that there are a wide variety of viewpoints here ... I'm still of the opinion that it's a terrible idea.

krisnye commented 13 years ago

But you never addressed the mutiple language environment issue. Your silence on that issue is basically saying that it's ok to have the exact same compatibility code duplicated at the tops of hundreds of files in a single project.

On Thu, Sep 22, 2011 at 8:47 AM, Jeremy Ashkenas < reply@reply.github.com>wrote:

I'm afraid that there are a wide variety of viewpoints here ... I'm still of the opinion that it's a terrible idea.

Reply to this email directly or view it on GitHub: https://github.com/jashkenas/coffee-script/issues/1363#issuecomment-2168654

jashkenas commented 13 years ago

Ideally, you only have a single copy of the compatibility code.

Also, you should feel free to strip it out yourself -- if your build tools are unable to compile your hundreds of scripts together into a single program ... go right ahead and have your build tools remove the bits you don't want from the top of the files.

It's just that CoffeeScript itself, as a compiler, should not be emitting JavaScript files that break in some environments, and run in others.

krisnye commented 13 years ago

You keep repeating the same thing without having actually ever comprehended either my earlier post concerning this or Rosenfelds. I will summarize the issue once again.

  1. My build process DOES merge hundreds of scripts into a single file.
  2. My source files are of MIXED languages. Some are coffeescript, some are javascript.
  3. Each source of either language may have a dependency on another source file of either language.
  4. The unified build process must convert everything to Javascript BEFORE merging them into a single file.
  5. After individual coffeescript files are compiled to javascript THEN the build process checks dependencies, orders files and merges them.

PLEASE understand this before you say yet again that "ideally we only have a single copy of the compatibility code".

In mixed language environments, You must convert to javascript before merging files. This means you have many individually compiled coffeescript programs, and therefore many instances of cruft.

On Thu, Sep 22, 2011 at 8:54 AM, Jeremy Ashkenas < reply@reply.github.com>wrote:

Ideally, you only have a single copy of the compatibility code.

Also, you should feel free to strip it out yourself -- if your build tools are unable to compile your hundreds of scripts together into a single program ... go right ahead and have your build tools remove the bits you don't want from the top of the files.

It's just that CoffeeScript itself, as a compiler, should not be emitting JavaScript files that break in some environments, and run in others.

Reply to this email directly or view it on GitHub: https://github.com/jashkenas/coffee-script/issues/1363#issuecomment-2168736

jashkenas commented 13 years ago

I understand entirely -- it sounds like your build tools are more than sophisticated enough to include something like this:

source = source.replace(/__hasProp[^\n]+\n/, '');
krisnye commented 13 years ago

No doubt. I have other hacks like that in place where required. It is my duty though to bring up the problem in the hopes that myself and other users like me (and according to this thread there are more than a few) will not all have to keep duplicating this hack in the future.

On Thu, Sep 22, 2011 at 9:17 AM, Jeremy Ashkenas < reply@reply.github.com>wrote:

I understand entirely -- it sounds like your build tools are more than sophisticated enough to include something like this:

source = source.replace(/__hasProp[^\n]+\n/, '');

Reply to this email directly or view it on GitHub: https://github.com/jashkenas/coffee-script/issues/1363#issuecomment-2169005

mvolkmann commented 13 years ago

Quoting Jeremy Ashkenas reply@reply.github.com:

I'm afraid that there are a wide variety of viewpoints here ... I'm still of the opinion that it's a terrible idea.

Clearly there is a tradeoff between trying to maximize readability of the generated code and trying to maximize the portability of the generated code. It seems to me that we can accomodate both through a command-line option. We'd be able to choose to sacrifice portability for readability in situations where we know the target platforms either support ES5 natively or a shimmed to do so, but the default would be to not assume that.

R. Mark Volkmann Object Computing, Inc.

rosenfeld commented 13 years ago

That's exactly the point. Jeremy, what I've asked you was exactly to describe what is the drawback of adding a non-default option for generating JS for ES5 compliant environments. Why do you think this is such a terrible idea? I'm sure the ones enabling such an option are smart enough for making sure the generated JS won't break on their chosen targets...

jashkenas commented 13 years ago

sigh -- clearly I'm outvoted on this one. I'll reopen the ticket, and we can entertain patches that add an option to target ES5.

michaelficarra commented 13 years ago

For what it's worth, I'm in agreement with you, @jashkenas. I don't think it's appropriate to add the flag. There's other provisions we take to be ES3/IE6 compliant, and those aren't going to be configured by this flag. And it's a half-assed solution anyway. We should be able to assume the existence of Function::bind under ES5, not make a helper that calls a bound Function::call on a function. That's not how people write their JS. CS is supposed to write JS that's better than what you would write yourself.

krisnye commented 13 years ago

There is no rush for a half assed solution. It is enough to just acknowledge that at some point a clean ES5 compile target is desirable.

myboundfunc = => @foo * 2

with the ES5 compile target should become

myboundfunc = (function(){ return this.foo * 2; }).bind(this);

On Thu, Sep 22, 2011 at 11:10 AM, Michael Ficarra < reply@reply.github.com>wrote:

For what it's worth, I'm in agreement with you, @jashkenas. I don't think it's appropriate to add the flag. There's other provisions we take to be ES3/IE6 compliant, and those aren't going to be configured by this flag. And it's a half-assed solution anyway. We should be able to assume the existence of Function::bind under ES5, not make a helper that calls a bound Function::call on a function. That's not how people write their JS. CS is supposed to write JS that's better than what you would write yourself.

Reply to this email directly or view it on GitHub: https://github.com/jashkenas/coffee-script/issues/1363#issuecomment-2170402

rosenfeld commented 13 years ago

I don't see any problems with half assed solutions to start with. It doesn't mean it can't be better. But I still find it better than reimplementing ES5 compliant functions. CoffeeScript (I'm talking about the implementation here) won't generate better code than I do manually. It can try, but it will hardly get to the point where its generated code will be better than my own hand crafted code.

For instance, consider this snippet:

f = () ->
  options = delayUpdate: true # just simplifying here
  updateResults() unless options.delayUpdate

CS generates this:

var f;
f = function() {
  var options;
  options = {
    delayUpdate: true
  };
  if (!options.delayUpdate) {
    return updateResults();
  }
};

While coding by hand I would write this instead:

function f() {
  var options = {delayUpdate: true}
  if (!options.delayUpdate) updateResults()
}

This is almost as small as CoffeeScript. I'm not blaming CS for this, I'm just saying that it won't ever be able to generate better code than hand craft ones by experienced JS developers.

eirikurn commented 13 years ago

I celebrate the decision to reevaluate this issue. I'm definitely +1 on giving developers support and trust to control their JS environment.

Regarding half-assed solutions, I believe we should also evaluate the other ES3-5 compromises and consider a road-map for version 2.0 with proper support for multiple compilation-targets. I think there can be ways to minimize community fragmentation if that is a worry. Btw, when will we create a standards committee? :)

@rosenfeld, I don't get your point about hand-crafted js code. Of course, "experienced" developers have always been hand-tuning code, even back to writing assembly instead of C. However, hand-tuning can be risky, and I prefer the security and productivity that CS provides to possible code size decreases.

With your example the difference is 48 bytes or almost 50% CS overhead. With uglify-js the difference drops to 18 bytes or around 25% CS overhead. Gzip has overhead and works better with bigger files, but when joined with jquery, the gzipped difference is just 5 bytes.

rosenfeld commented 13 years ago

Hi @eirikurn, there was no point. I was just commenting a statement from @michaelficarra: "CS is supposed to write JS that's better than what you would write yourself". I guess I wasn't clear on that before, sorry :)

nibblebot commented 12 years ago

+1 for --es5 compile target to use .call, .bind! if anyone is working on a fork for pull request, can you paste link here?

michaelficarra commented 12 years ago

@nibblebot: #1408, https://github.com/michaelficarra/CoffeeScriptRedux

krisnye commented 12 years ago

Congratulations on getting funding @michaelficarra. So I assume that -es5 target will be an option ;). Are you planning on integrating most of your ideas from your coffee-of-my-dreams project?

michaelficarra commented 12 years ago

@krisnye: Yes, there will be support for generating JS that assumes an ES5 execution environment. The coffee-of-my-dreams project will be a fork of the funded project after it is completed. It will simply add many new and mostly functional features to CoffeeScript, in the same spirit as gkz/LiveScript.

vinyll commented 11 years ago

+1 for having a --es5 export option. I don't need any IE support and the JS code generated is not as clean as it could be.