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

ES5 bind #1408

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@michaelficarra
Copy link
Collaborator

michaelficarra commented Jun 1, 2011

bind helper now uses native Function::bind when it is available and our custom version, supplied for environments without Function::bind, is now ES5 compliant with respect to construction (new) of bound functions (=>)


You may remember that this was on master until @jashkenas decided to take it off without a good alternative for the errant test case and broken bind function.

I don't think we should have bound function literals

=> ...

if they don't behave exactly as

(-> ...).bind(this)

would in an environment with ES5's Function::bind.

`bind` helper now uses native Function::bind when it is available and
our custom version, supplied for environments without Function::bind,
is now ES5 compliant with respect to construction (`new`) of bound
functions (`=>`)
@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented Jun 1, 2011

Thanks for putting this on a branch.

@michaelficarra

This comment has been minimized.

Copy link
Collaborator

michaelficarra commented Jun 1, 2011

Just doing as you suggested. Thanks for not immediately dismissing it if you didn't agree with it before. Also, did you notice that JS file diffs are now treated as binary diffs are in other projects? I remember you asking for this a long time ago on a blog post when they started doing it for some mobile application binary. Looks like they finally did it.

@TrevorBurnham

This comment has been minimized.

Copy link
Collaborator

TrevorBurnham commented Jun 1, 2011

Wow, that's really cool about GitHub suppressing the JS diffs.

As to the main topic here: The tradeoff is more standard behavior at the cost of a larger __bind helper (69 bytes minified vs. 254 bytes, under Google Closure Compiler), right? That sounds worth it to me. People who are trying to squeeze bytes would use self = this over => anyway. Or are there concerns about the proposed behavior?

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented Jun 1, 2011

There are concerns about the proposed behavior. I want to have a chance to give it a thorough look.

@josh

This comment has been minimized.

Copy link
Contributor

josh commented Jun 1, 2011

@michaelficarra Had to do a bunch of crazy sniffing stuff to detect Coffee generated JS

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented Jun 1, 2011

@josh What did you end up using for your sniffing implementation?

@josh

This comment has been minimized.

Copy link
Contributor

josh commented Jun 1, 2011

https://gist.github.com/1002993 - all the language detection code should be open sourced soon.

(Sorry for derailing the topic on this pull)

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented Jun 1, 2011

Fascinating -- I bet that code gets you close to a 100% detection rate, for any nontrivial committed files. Nicely done.

@ghost ghost assigned jashkenas Jun 27, 2011

@michaelficarra

This comment has been minimized.

Copy link
Collaborator

michaelficarra commented Aug 17, 2011

@jashkenas: ping

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented Aug 17, 2011

Thanks for the ping, but I'm afraid I won't get a chance to take a look at this too soon -- imminent deadlines.

michaelficarra added some commits Jun 1, 2011

`bind` helper now uses native Function::bind when it is available and
our custom version, supplied for environments without Function::bind,
is now ES5 compliant with respect to construction (`new`) of bound
functions (`=>`)
Merge branch 'es5_bind' of github.com:jashkenas/coffee-script into es…
…5_bind

Conflicts:
	lib/coffee-script/nodes.js
	src/nodes.coffee
@michaelficarra

This comment has been minimized.

Copy link
Collaborator

michaelficarra commented Aug 26, 2011

Great, something I did made other commits appear as part of this pull request. Anyway, the real comparison is here: master...es5_bind

@jashkenas: ping?

@tpetry

This comment has been minimized.

Copy link

tpetry commented Sep 19, 2011

Wouldn't it be possible to add an es5 compiler flag to CS? So the compiler could use the NATIVE bind function without needing to declare any wrapper function. Whenever you activate this flag and you'll deploy the compiled js you have to be sure that the target js runtime has es5 support or you should an es5-shim.

Ths would make the compiled code much more readable especially when compiling for nodejs. Maybe many more compiled statements could make use of the es5 features.

@michaelficarra

This comment has been minimized.

Copy link
Collaborator

michaelficarra commented Sep 19, 2011

@tpetry: CoffeeScript has only one compilation target: lowest-common-denominator ES3. It would be nice to be able to have multiple compilation targets, but the code generation is so deeply mixed with the AST that it'd be impossible even if we wanted to do it. I'm planning to do it the right way, but it'll be a while before that's available.

@jashkenas

This comment has been minimized.

Copy link
Owner

jashkenas commented Sep 19, 2011

And for the record -- as discussed in other issues I can't recall at the moment -- targeting lowest-common-denominator JavaScript is very much by design. Separating the code gen from the AST may well be a worthwhile refactor, but we would still want to only ever compile to JavaScript that runs everywhere.

@tpetry

This comment has been minimized.

Copy link

tpetry commented Sep 19, 2011

@michaelficarra: Isolating the code generation and AST sounds great.

@jashkenas: ok. Only thought it would be a great while one of the big goals of CS is to generate readable JS, and the actual ___bind() approach is much more unintuitive than Function.bind() introduced with ES5

@showell

This comment has been minimized.

Copy link

showell commented Sep 19, 2011

It's pretty easy to extract an AST from CS internals, but I agree that it's hard to reuse the Code Generation code.

FWIW, see this program, which traverses the Node objects from CS and writes out a JSON representation of the AST that other programs can use without mucking around in CS internals:

https://github.com/showell/CoffeeNodesToJson

I like that LCD JS is the primary target of CS, but I also like the idea of opening up the AST for folks with different targets in mind. CS could eventually compile to JVM bytecode, for example, which would allow Ruby programmers to write more CS (via JRuby and friends). Also, Dart/Dash.

@michaelficarra

This comment has been minimized.

Copy link
Collaborator

michaelficarra commented Apr 8, 2012

@vendethiel

This comment has been minimized.

Copy link
Collaborator

vendethiel commented May 3, 2013

closing for #1363 (for now)

@vendethiel vendethiel closed this May 3, 2013

@GeoffreyBooth GeoffreyBooth deleted the es5_bind branch May 2, 2017

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