rescript-compiler icon indicating copy to clipboard operation
rescript-compiler copied to clipboard

Request for @functionExpression attribute to opt-out from arrow functions

Open DZakh opened this issue 1 year ago • 10 comments

After the PR https://github.com/rescript-lang/rescript-compiler/pull/6945 function expressions were changed to arrow functions to improve readability and bundle-size. But it also broke this and arguments binding to the function context, which could be accessed via %raw("this") when needed for some advanced logic.

I need it for rescript-schema, so I think introducing an attribute @functionExpression to opt-out from arrow functions would be a good solution.

DZakh avatar Sep 30 '24 08:09 DZakh

@cknitt @cometkim

DZakh avatar Sep 30 '24 08:09 DZakh

Doesn't @meth work?

We also have @this binding. Afaik, it works for both regular function and FFI

cometkim avatar Sep 30 '24 09:09 cometkim

Doesn't @meth work?

We also have @this binding. Afaik, it works for both regular function and FFI

I'll check

DZakh avatar Sep 30 '24 09:09 DZakh

Unfortunately, they do nothing here: https://rescript-lang.org/try?version=v12.0.0-alpha.3&code=DYUwLgBAZhC8EAoCUcB8EDeAoCFSQHM5EABMACwEsBnCAQxVnQGEB7AO2tdADphWCCAEQAVENUhCIAamn0kOCIKF0hC3PgjliCEgFtw2hmghtO3EHwHCxEiFNnzF5YaoUBfLEA

or here: https://rescript-lang.org/try?version=v12.0.0-alpha.3&code=DYUwLgBAZhC8EAoCUcB8EDeAoCEACYAFgJYDOOEokA5nBAIZoQDCA9gHamugB0wr1BACIAKiFKQhEANTSGSCoKH0hC3HgC24QhSoRCdRrHRtO3EHwHCxEiFNnyKhYSoUBfLEA

fhammerschmidt avatar Sep 30 '24 09:09 fhammerschmidt

I've been exploring this for a bit. rescript-schema seems to be a somewhat special case. I believe most apps have a more explicit and idiomatic solution for binding context without relying on JS this.

ReScript's @this attr support is only compatible with the Js_OO.Callback type, not plain functions because that is the safe way to bind this in "callbacks".

It wouldn't be hard to implement for plain functions either, since there are already primitive implementations, but it would be inherently an unsafe way to write code, so even if we add it, it should be marked as an "unsafe" feature.

cometkim avatar Sep 30 '24 21:09 cometkim

I've been exploring this for a bit. rescript-schema seems to be a somewhat special case. I believe most apps have a more explicit and idiomatic solution for binding context without relying on JS this.

Agreed.

It is important that we get rescript-schema to work with v12 though. Is that not feasible at all without this, @DZakh? If so, then I think we should add the requested feature.

cknitt avatar Oct 04 '24 08:10 cknitt

It seems like there's a lot of this dependency, and removing it and going back to the type t pattern would require a lot of boilerplate and some breaking changes. rescript-schema looks like they're trying to avoid that and provide a more JS-friendly API.

My suggestion is to expose a primitive API instead of attributes. We already have the necessary implementations; %unsafe_to_method. If we provide a proper namespace for it, we can do something like this:

Js.Unsafe.thisCall((this, arg1, arg2) => {
  ...
})
// -> (('this, 'a, 'b) => 'result) => (('a, 'b) => 'result)

cometkim avatar Oct 04 '24 12:10 cometkim

@DZakh Would @cometkim's suggestion work for you?

This is something that you could already test now. %unsafe_to_method is currently exposed as Js_OO.unsafe_to_method.

cknitt avatar Oct 10 '24 08:10 cknitt

Js_OO.unsafe_to_method works for me.

DZakh avatar Oct 16 '24 13:10 DZakh

Should we document this somehow?

fhammerschmidt avatar Oct 16 '24 14:10 fhammerschmidt