Skip to content

Remove lazy keyword #6241

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

Closed
cknitt opened this issue May 3, 2023 · 5 comments · Fixed by #6342
Closed

Remove lazy keyword #6241

cknitt opened this issue May 3, 2023 · 5 comments · Fixed by #6342
Milestone

Comments

@cknitt
Copy link
Member

cknitt commented May 3, 2023

It seems the lazy keyword inherited from OCaml is a not commonly used or known in ReScript.

We should consider removing this keyword, it is always possible to use Lazy.from_fun instead if one wants to use OCaml's Lazy module.

And possibly a Lazy module with a more ReScript-like API could be added to Core.

@TheSpyder
Copy link
Contributor

By what metric was it "not commonly used or known"? I have ~50 usages across a decent size codebase. Not a ton, I'll admit, but still.

I've done manual thunking for years in JS, it was nice to have explicit syntax for it.

@glennsl
Copy link
Contributor

glennsl commented Feb 22, 2024

Can you show a usage example, with and without lazy? Just curious as I've never thought to use it myself.

@TheSpyder
Copy link
Contributor

TheSpyder commented Feb 22, 2024

The linked TypeScript code is using options, with the thunk representing the "if none" value but we want to avoid the computation in the "some" case. Basically the same as a lazy. It might help to see the definition.

As a replacement for getOrThunk in our ReScript code, we created orLazy:


let orLazy = (opt, v) =>
  switch opt {
  | Some(o) => o
  | None => Lazy.force(v)
  }

A simple example is when rendering the editor in a test, we have an optional schema and if one isn't provided we create a default.

let toEditor = (~schema=?, nodes) =>
    Markup.toEditor(~schema=schema->Option.orLazy(lazy DefaultSchema.make()), nodes)

And yes, absolutely, we could do Markup.toEditor(~schema=schema->Option.getOrThunk(() => DefaultSchema.make()), nodes). But lazy is very clear about what's happening.

@glennsl
Copy link
Contributor

glennsl commented Feb 22, 2024

Thanks for the explanation! Just to give a fresh eyes perspective on it:

I think the clarity here comes mostly from the function being named orLazy, because the laziness is mostly a property of the function, not of the value passed to it. Lazy values serve a different purpose than thunks, and I think that's important for communicating intent. A lazy value, once forced, is stored and never re-computed. But that's not a property that is really employed here. It is only called once, but that's pretty obvious from the name of the function, so there's no need to use a lazy value to guarantee it. I actually find that makes its use here slightly less clear, because it leads me to start second-guessing the intent.

It's not hard to imagine a hypothetical scenario where a lazy value is used by several calls to orLazy though, where you only want it computed once, but I've never come across such a scenario myself, that I can recall.

@TheSpyder
Copy link
Contributor

ah yes, I do understand the difference now that you mention it.

It's hard to see in our code whether we depend on the caching nature of lazy values or just leverage them as single-use thunks as in my example.

I suppose as long as we still have the lazy type we can force the occasions we need caching to use Lazy.from_fun which is more or less what the lazy values compiled to in JS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants