Skip to content

Feature suggestion: syntax for Js.Dict.t literals #6301

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
johnridesabike opened this issue Nov 20, 2020 · 13 comments
Closed

Feature suggestion: syntax for Js.Dict.t literals #6301

johnridesabike opened this issue Nov 20, 2020 · 13 comments
Labels
stale Old issues that went stale

Comments

@johnridesabike
Copy link

I've been working with code that uses "hash map" style JS objects, and the ReScript syntax for declaring them can be tedious. (Or rather, the lack of syntax to declare them is tedious.)

Consider this JS:

const data = {
  a: foo,
  b: bar,
  c: baz,
};

Compared with this ReScript:

let data = Js.Dict.fromArray([
  ("a", foo),
  ("b", bar),
  ("c", baz),
])

That's a lot of extra parentheses and brackets to express the same thing. When you inline it somewhere (such as inside a function argument), then it's even more noisy and less readable.

My suggestion is to include a dedicated syntax similar to Js.t objects. Maybe something like this:

@dict
let data = {
  "a": foo,
  "b": bar,
  "c": baz,
}

Would this make sense to add?

@bloodyowl
Copy link
Collaborator

I think we could even have a generic system for simple, literal keys:

// Js.Dict
dict{
  "a": 1,
  "b": 2,
  "c": 3,
}

// Belt.Map.String
map{
  "a": 1,
  "b": 2,
  "c": 3,
}

// Belt.Map.Int
map{
  1: 1,
  2: 2,
  3: 3,
}

set{1, 2, 3} // Belt.Set.Int
set{"foo", "bar", "baz"} // Belt.Set.String

@johnridesabike
Copy link
Author

That would be cool if it was possible, although that would have to still desugar/compile to function calls, right? Js.Dict can potentially compile to an object literal, which seems much simpler.

@bloodyowl
Copy link
Collaborator

I think I saw an issue on the compiler that @bobzhang opened some time ago that discussed the possibility to convert ˋfromArray` calls with literal to the data structure itself. I think it'd be fairly easy for dicts, but not sure about Belt data structures.

@jfrolich
Copy link
Contributor

@bloodyowl: Love this approach!

@tsnobip
Copy link
Member

tsnobip commented Feb 11, 2021

@bloodyowl if calls to Js.Dict.fromArray with a literal array would be compiled directly to the given json object then it would totally make sense to have a dedicated syntax. But I wouldn't mind the slight runtime cost meanwhile.
I couldn't find the issue though, are you talking about #2320 ? It's more than 3 years old, I hope it'll get more love now ^^

@jaidetree
Copy link

@bloodyowl I would very much be in favor of that.

My use case in case there's a better way

module Icon = {
  @react.component
  let make = (~icon: option<string>=?) => {
    <img src={icon->Option.getWithDefault("/img/default-icon")} />
  }
}

module LabelWithIcon = {
  @module external hobbyIcon: string = "./hobby.svg"
  @module external memoryIcon: string = "./memory.svg"
  @module external standardIcon: string = "./standard.svg"
  let icons = Js.Dict.fromArray([
    ("Hobby", hobbyIcon),
    ("memory", memoryIcon),
    ("standard", standardIcon),
  ])

  @react.component
  let make = (~id: string, ~label: string) => {
    <label><Icon icon={icons->Js.Dict.unsafeGet(id)} />{label->React.string}</label>
  }
}

My rationale is that explicitly typing all the possible ids with a Record would be rather tedious. Additionally by definition of this type it does not really matter if an icon displays or not, it's a nice-to-have, but no one is going to miss them if they don't display. The fromArray works but it's a bit of a roundabout way to get there so a direct syntax would be awesome!

@stale
Copy link

stale bot commented May 28, 2023

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 stale bot added the stale Old issues that went stale label May 28, 2023
@stale stale bot closed this as completed Jun 10, 2023
@tsnobip
Copy link
Member

tsnobip commented Jun 11, 2023

I would still be in favor of having this feature.

@zth
Copy link
Collaborator

zth commented Jun 11, 2023

I also find this interesting. @tsnobip maybe open a new issue for it in the compiler repo?

@cknitt
Copy link
Member

cknitt commented Jun 12, 2023

I'll transfer this one to the compiler repo.

@cknitt cknitt transferred this issue from rescript-lang/syntax Jun 12, 2023
@cknitt cknitt reopened this Jun 12, 2023
@cknitt cknitt removed the stale Old issues that went stale label Jun 12, 2023
@CarlOlson
Copy link

A couple of thoughts:

  1. Having syntax that expands to Belt or Js will elevate those libraries, not all users want to use those implementation.
  2. Js.Dict in particular is not safe. For example, using a Js.Dict with keys from a client, the client could set __proto__. Most devs wouldn't consider this kind of thing. ES6 Maps would be preferable.
  3. This could be a good test case for an official PPX alternative. At the very least, it should be configurable due to the above reasons.

Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Old issues that went stale label Oct 15, 2024
@zth
Copy link
Collaborator

zth commented Oct 15, 2024

Dedicated syntax was added recently.

@zth zth closed this as completed Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Old issues that went stale
Projects
None yet
Development

No branches or pull requests

8 participants