Skip to content
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

Add dict literal syntax #6617

Closed
wants to merge 2 commits into from

Conversation

JonoPrest
Copy link
Contributor

Hi @zth!

I've got a somewhat working implementation of the dict literal syntax. #6545

So far this branch is a bit of a playground I was using to get familiar with the parser/printer. Could I get some feedback? I'm also wanting to add a number of tests that to validate correct and incorrect values. Is it possible to write a test that expects a failure if two different types are passed into the right hand side expression of the key value for example?

Still todo:

  • comments in printer always go to the end of the dict instead of above the row item.
  • perhaps make more reusable functions for similar processing of lists etc?

@JonoPrest
Copy link
Contributor Author

For reference this is what it should parse as:

let demoDict = dict{"a": 1, "b": 2}
//Compiles to
let _ = Js.Dict.fromArray([("a", 1), ("b", 2)])

let _ = dict{...demoDict, "b": 3, "c": 4}
//Compiles to
let _ = Js.Dict.fromArray(Belt.Array.concatMany([Js.Dict.entries(demoDict), [("b", 2), ("c", 4)]]))

let _ = dict{...demoDict}
//Compiles to
let _ = Js.Dict.fromArray(Js.Dict.entries(demoDict))

@JonoPrest
Copy link
Contributor Author

It seems I've broken some tests 🤔, will see if I can debug it 👍🏼

@JonoPrest JonoPrest force-pushed the add-dict-literal branch 3 times, most recently from 639a1fa to 15b8c44 Compare February 11, 2024 15:01
Remove comments

Remove unused collect function

Remove print

Fix dead code
@zth
Copy link
Collaborator

zth commented Feb 11, 2024

Awesome stuff! Having a look soon.

@cknitt
Copy link
Member

cknitt commented Feb 29, 2024

It would be great if we could get

let demoDict = dict{"a": 1, "b": 2}

to compile to

var demoDict = {
  a: 1,
  b: 2
};

instead of

var demoDict = Js_dict.fromArray([
      [
        "a",
        1
      ],
      [
        "b",
        2
      ]
    ]);

@zth
Copy link
Collaborator

zth commented Feb 29, 2024

It would be great if we could get

let demoDict = dict{"a": 1, "b": 2}

to compile to

var demoDict = {
  a: 1,
  b: 2
};

instead of

var demoDict = Js_dict.fromArray([
      [
        "a",
        1
      ],
      [
        "b",
        2
      ]
    ]);

This is the goal, and it's actually already implemented in a separate branch.

@JonoPrest
Copy link
Contributor Author

It would be great if we could get

let demoDict = dict{"a": 1, "b": 2}

to compile to

var demoDict = {
  a: 1,
  b: 2
};

instead of

var demoDict = Js_dict.fromArray([
      [
        "a",
        1
      ],
      [
        "b",
        2
      ]
    ]);

@cknitt thanks for the feedback! I had a chat with Gabriel and think ultimately there will be some sort of magic dict constructor function that gets called. And then the js compiler can convert it literally into a js object and perhaps use js spread syntax etc.

I'll leave this PR as-is in a draft until some of the bigger decisions have been reached.

@zth
Copy link
Collaborator

zth commented Mar 5, 2024

Here's the PR for reference: #6538

@cknitt
Copy link
Member

cknitt commented May 26, 2024

@JonoPrest I guess this can be closed in favor of #6774?

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 this pull request may close these issues.

None yet

3 participants