syntax icon indicating copy to clipboard operation
syntax copied to clipboard

Optimize ref.contents to something more user friendly

Open Huxpro opened this issue 5 years ago • 18 comments

Personally, I understood that contents is the mutable field name of the record returned by ref constructor in the OCaml std libraries, however:

  1. it's quite foreign to JavaScript community.
  2. I've also heard people complain that "contents" being a plural could be a significant source of typo for non-native speakers.

I'm not asking to bring a syntax sugar back (though I'm happy to have * ref...). A workaround could be as simple as replacing that name to something better. current, value, deref are all good candidates.

Alternatively, to not break existing .contents code, adding a helper function e.g. deref() to the Pervasive may be sufficient.

Huxpro avatar Aug 24 '20 06:08 Huxpro

I agree it's a case where it kinda deserves being a special feature but instead it got relegated a reduction of another feature.

Not sure we should move away from that particular struct. I think it's optimized specially in the compiler? Though we can always change that...

chenglou avatar Aug 24 '20 06:08 chenglou

If we want to move away from contents, current would be nice as it would match the definition of a React ref (https://github.com/reasonml/reason-react/blob/795dc08e2e66a34a7bbd515bd457b5a70b7d57f7/src/React.re#L42).

(I would prefer to keep a special syntax like ^ though.)

cknitt avatar Aug 24 '20 07:08 cknitt

@chenglou Since ReScript now owns the syntax and the compiler, I believe it's trivial to rename it? I will wait @bobzhang to answer this but I'm more than happy to help. Alternatively, second what I said, it might be sufficient to provide a deref() which essentially an inlined contents access.

@cknitt Yep that's why it's on my candidates list (and FYI the value one is coming from Vue). The only concern to current is that then you can't figure out if a ref in ref.current is a language ref or React ref by a glance.

Personally I like deref most since it's descriptive and a natural dual of ref. The JS proposal of adding boxed type is also using deref in the example.

const obj = { hello: "world" };
const box = Box(obj);
assert(obj === box.deref(), "boxes can deref the full object");

Huxpro avatar Aug 24 '20 19:08 Huxpro

@cknitt do you have lots of pre-existing .contents in your codebase?

IwanKaramazow avatar Aug 24 '20 19:08 IwanKaramazow

ref is not an abstract type in pervasives. Its definition is

type 'a ref = { mutable contents : 'a}

So we can not really hide .contents unless we make it an abstract type. In OCaml, there's an sugar like this !x for dereferences, but ! is redefinable and mostly used as not in JS. I propose that we introduce a non-redefinable sugar to make it more accessible

bobzhang avatar Aug 25 '20 01:08 bobzhang

I am interested in why it's not user friendly? I think the extra syntax in OCaml and Reason is just adding more complexity to something that is actually quite simple. A record with a mutable field also closely maps how to create mutable values in JavaScript.

jfrolich avatar Aug 25 '20 01:08 jfrolich

@bobzhang Yeah, but I mean, if the compiler doesn't need to be compatible with OCaml's Pervasives, then we can just rename it right? If we do want to be compatible, then we can just prelude a deref() w/o bothering inventing a new syntax.

@jfrolich It's mostly not friendly to the JS community that ReScript is targeting, where people doesn't need to care about such theoretical cleanness and rather prefer something more mappable from the JS var a = 1 or the React ref that they've already knew.

Huxpro avatar Aug 25 '20 03:08 Huxpro

“It does not need to be” but we need large benefits to justify. There’re some benefit to keep contents when you are doing a nested pattern match. For normal js users, if we provide three sugars for ref/deref/modify it should cover most use cases?

Xuan Huang [email protected]于2020年8月25日 周二上午11:02写道:

@bobzhang https://github.com/bobzhang Yeah, but I mean, if the compiler doesn't need to be compatible with OCaml's Pervasives, then we can just rename it right? If we do want to be compatible, then we can just prelude a deref() w/o bothering inventing a new syntax.

@jfrolich https://github.com/jfrolich It's mostly not friendly to the JS community that ReScript is targeting, where people doesn't need to care about such theoretical cleanness and rather prefer something more mappable from the JS var a = 1 or the React ref that they've already knew.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rescript-lang/syntax/issues/115#issuecomment-679482462, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFWMKYDSXHNT6OUDUAC5QDSCMSVBANCNFSM4QJEDO4A .

-- Regards -- Hongbo Zhang

bobzhang avatar Aug 25 '20 03:08 bobzhang

@jfrolich It's mostly not friendly to the JS community that ReScript is targeting, where people doesn't need to care about such theoretical cleanness and rather prefer something more mappable from the JS var a = 1 or the React ref that they've already knew.

I am totally not talking about theoretical cleanliness. I am talking about the equivalent in JS. In JS you would also do it like this if you want to pass around a value that is mutable (this pattern is for instance used in React's useRef):

let value = {
   contents: 1;
}

function mutateValue(value) {
  value.contents = 2;
}

This use case:

let a = 1;
a = 2;

is already covered with this ReScript code (rebinding)

let a = 1;
let a = 2;

jfrolich avatar Aug 25 '20 04:08 jfrolich

The gap with JS is that ReScript doesn't have assignment but it does have rebinding. This is due to ReScript bindings being immutable. Adding sugar to this with ref syntax doesn't make it a whole lot clearer to new users I think (I might be wrong).

jfrolich avatar Aug 25 '20 04:08 jfrolich

@cknitt do you have lots of pre-existing .contents in your codebase?

No, we are using ^ everywhere, and we do not have any other records with a field named contents.

cknitt avatar Aug 25 '20 05:08 cknitt

Rescript newbie here but.... why not just make ":=" be sugar to append ".contents" on both sides "IF" value is a ref? What I'm I missing??

ruifortes avatar Mar 02 '21 11:03 ruifortes

Hey. Not sure what you mean but you can't know whether something is a ref purely syntactically

chenglou avatar Mar 02 '21 11:03 chenglou

ahh..ok. I guerss that's why there's also the need those ".*" operators for floats. But I don't get it though... why can't you know the type in a language like this?!

ruifortes avatar Mar 02 '21 12:03 ruifortes

The .* is a tradeoff that could have been solved. That'd be a different issue.

We do know the types. And we can theoretically update things in the compiler. But the topic was on a syntactical transform.

chenglou avatar Mar 02 '21 13:03 chenglou

...still over my head. Is there documentation about the general pipeline of rescripts trans, com, whateverstep, ...piling for dummies?

ruifortes avatar Mar 02 '21 14:03 ruifortes

Let's avoid pinging everyone here =) Mind starting a thread on the forum?

chenglou avatar Mar 02 '21 16:03 chenglou

Sure. Thanks :-)

ruifortes avatar Mar 02 '21 20:03 ruifortes

The rescript-lang/syntax repo is obsolete and will be archived soon. If this issue is still relevant, please reopen in the compiler repo (https://github.com/rescript-lang/rescript-compiler) or comment here to ask for it to be moved. Thank you for your contributions.

stale[bot] avatar May 28 '23 23:05 stale[bot]