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

Question: Runtime representation #24

Open
i-am-the-slime opened this issue Jul 10, 2023 · 5 comments
Open

Question: Runtime representation #24

i-am-the-slime opened this issue Jul 10, 2023 · 5 comments

Comments

@i-am-the-slime
Copy link
Contributor

Does the runtime representation look like this:

{ there: 1, missing: undefined }

Or

{ there: 1 }

?

@paluh
Copy link
Owner

paluh commented Jul 10, 2023

From JS perspective is there any difference?

@jvliwanag
Copy link

let x = {};
let y = {foo: undefined};

x.hasOwnProperty("foo"); // false
y.hasOwnProperty("foo"); // true

Object.keys(x); // []
Object.keys(y); // ["foo"]

@paluh
Copy link
Owner

paluh commented Jul 10, 2023

Ah yeah. So it can break someones equality assumption etc. I'm a bit confused - can we keep this ambiguity - it seems handy. What do you think Guys?

So for example at the end the type { opt :: Opt Null } contains three values really:

{ opt: null }, { opt: undefined } , {}

@i-am-the-slime
Copy link
Contributor Author

i-am-the-slime commented Jul 11, 2023

Well I presume there's a big difference memory-wise. I use this to construct some potentially very large records but practically tiny records in FFI. The reason I noticed is that react was complaining about a prop name that didn't exist, but I wasn't really passing that prop (Bing is quite a bit worse at generating FFI than ChatGPT by the way).

So it was actually nice to find out that that key I didn't have example coverage was wrong.
However I would prefer the {} version. I was wondering if it came from this lib, but it's probably from Record.Builder:

subContent 
   p kids. Coerce p DropdownMenuSubContentProps  ToJSX kids  p  kids  JSX
subContent props kids = React.element subContentImpl
  $ (coerce props  DropdownMenuSubContentProps) # RB.build
      ( RB.insert (Proxy  _ "children") kids
      )

@paluh
Copy link
Owner

paluh commented Jul 11, 2023

That is strange. coerce props is noop (just unsafeCoerce) and RB.build uses copyRecord which actually iterates over existing JS object props.

The lib can add a prop to the record if you build it using:

recordWithAField :: { opt :: Opt Null }
recordWithAField = { opt: undefined }

But I would like to leave it as it seems handy - I can document this behavior. What do you think?
We can also remove first class undefined support and encourage merging. I'm not sure.

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

No branches or pull requests

3 participants