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

Allowing records and tuples to contain functions #390

Closed
danieltroger opened this issue Sep 13, 2024 · 16 comments
Closed

Allowing records and tuples to contain functions #390

danieltroger opened this issue Sep 13, 2024 · 16 comments

Comments

@danieltroger
Copy link

danieltroger commented Sep 13, 2024

Right now functions are not allowed in records and tuples, see from the Readme

Records and Tuples may only contain primitives and other Records and Tuples. Attempting to create a Record or Tuple that contains an Object (null is not an object) or a Function throws a TypeError.

Excuse my ignorance, but why?
I've been using the R&T polyfill for a while now and continuously bump into R&T not working for a usecase because they're not allowed to contain functions. Object.freeze works on functions, so I'd really like to be able to put them into especially records.

For example I'd want to have an array like this

const onboardingSteps = [
  #{ title: "Publish a collection", icon: RocketLaunch },
  #{ title: "Add Content to your grid", icon: Image },
  #{ title: "Duplicate a product", icon: CopySimple },
  #{ title: "Claim your free call", icon: PhoneCall },
  #{ title: "Give us your feedback", icon: HandHeart },
];

Where icon is a react component, which is a function.

That way I could just render this with

onboardingSteps.map(step => {
  // Skip completed steps
  if(completedSteps.includes(step)) return;
  // render step
  const { title, icon } = step;
  return <div><Icon />{title}</div>
});

And not have to write a comparison function that could break as more properties are added to each step and is far less elegant.

@Maxdamantus
Copy link

Maxdamantus commented Sep 13, 2024

Some previous discussions:
#292
#206

From memory, I guess the TLDR from 292 would be:

  1. allowing objects/functions as members means that R/T values can't be assumed to be "deeply immutable"
  2. and it would require typeof #[{}] === "object" (Should Records and Tuples be objects, instead of primitives? #201)
  3. and typeof #[{}] === "object" raises questions around object identity WRT handling of -0 and +0 (in particular, #[-0] and #[+0] would be different (!Object.is(...)) but equal (... === ...) objects)

Personally, I don't think 1 is generally useful and in the special cases where it is desired (eg, structuredClone()), a check for deep immutability can be performed. And I think 2 and 3 are reasonable tradeoffs for the advantages of proper composite values (where object values are also values that can be composited), though some TC39 members seem to disagree.

More recently, the treatment of equality of R/T values has also been reconsidered, meaning use cases such as your completeSteps.includes would not work anyway, though judging by the reactions to this comment (#387 (comment)) I suspect/hope popular opinion means this proposal will maintain its equality semantics, even if it means it will never be adopted 😢.

@danieltroger
Copy link
Author

Thank you for the links, interesting discussions.
Although I'm still too little into how JS works specifics to completely understand your answer.

  1. Why can't R/T values be assumed to be "deeply immutable" if one allows adding functions? Just don't allow the functions to change.
  2. Why does allowing a record to contain an object force it to become typeof object? Why can't typeof #[{}] still be "tuple"?

I reaaaalllyy hope they don't give up the equality part. Agree that that's what makes this proposal interesting. The function thing is not such a biggie, I could always just use a WeakMap and symbols, it just would be nice if I didn't have to.

@ljharb
Copy link
Member

ljharb commented Sep 13, 2024

It’s not just about changing, it’s about being a communications channel. A frozen function can still repeatedly return a mutable object.

@acutmore
Copy link
Collaborator

Why can't R/T values be assumed to be "deeply immutable" if one allows adding functions? Just don't allow the functions to change.

Functions are also objects. So this would be possible:

const aFunction = () => {};
aFunction.staticProp = 1;
const tuple = #[aFunction];
aFunction.staticProp = 2;

This would not meet many people's definition of 'deeply immutable'. It is deeply immutable if the function is considered only as 'a reference to a function' - in that regard it isn't changing.

I do agree that not allowing functions directly limits the use cases. Allowing functions opens up use cases, but introduces mutable objects. Different people weigh up those two sides differently.

Why does allowing a record to contain an object force it to become typeof object? Why can't typeof #[{}] still be "tuple"?

There is existing code that assumes any value where the typeof thatValue is not "object" or "function" is inert data that does not need to be sanitised when providing certain types of sandboxing. This is why if R&T have "record" or "tuple" as their typeof, then they can only contain primitive values. If we drop the custom typeof and they are "object" then support for -0 also needs to be dropped.

BTW if you are interested I tried to make a website that helps explain the interconnectedness of the R&T design space: https://acutmore.github.io/record-tuple-laboratory/

@danieltroger
Copy link
Author

Thank you for the playground, that's hilarious. Will look at it more later.
And thank you for the explanation, I understand it better now!

Functions are also objects. So this would be possible:

const aFunction = () => {};
aFunction.staticProp = 1;
const tuple = #[aFunction];
aFunction.staticProp = 2;

Okay and could that be fixed by only allow adding frozen functions into R&T? This way one could just wrap the functions in Object.freeze and then it would work. Much smaller annoyance than having to create a WeakMap of symbol to function and always look stuff up there.

And the code that assumes records and tuples to be inert would still be correct. You'd only have to add a check when creating records & tuples that functions are frozen.

Arrrgggh ok I already realise that this wouldn't work because "deep-freezing" isn't easy and one could have another object already as property on the function that one then changes.

And it's not easy to check if functions are "unmodified", meaning they don't contain any other properties than what's needed to execute them and then only allow such functions?

@mhofman
Copy link
Member

mhofman commented Sep 13, 2024

Freezing, even deeply, wouldn't be sufficient. One there is no guarantee that a frozen function is not in fact a proxy, which records and tuples would guarantee. Two, as mentioned previously, the function would be free to return mutable objects, which would be against what some consider as deeply immutable

@demurgos
Copy link

There are also other differences between functions and records or tuples:

  • functions have a non-null prototype
  • functions may carry a closed over (mutable) environment

R/T as primitives (with their lack of prototype) means very good support for being sent across realms or serialized/structured cloned.

@mhofman
Copy link
Member

mhofman commented Sep 13, 2024

There may be a possibility that could be acceptable to stakeholders:

  • Functions when being used to create a record/tuple, would automatically be "wrapped" (similar to bound): a new frozen function object would instead be used in the R/T. That wrapper simply calls the original function.
  • R/T containing functions would never be equal to another R/T, even if that R/T has its nested function wrapping the same original function.
  • R/T must have a typeof 'object', as a primitive shouldn't enable a communication channel on its own
  • There is a predicate to know if a R/T contains a nested function
  • The ShadowRealm callable boundary could let these R/T through by replacing the function with a callable boundary wrapper.

This is sort of a variation of the "Box" idea, but without equality

Edit: The reason this may be acceptable is that an object containing a function is likely already considered today as not being pure.

@danieltroger
Copy link
Author

One there is no guarantee that a frozen function is not in fact a proxy, which records and tuples would guarantee

Oh I never thought about this, is it not possible to proxy records and tuples at all?

@mhofman
Copy link
Member

mhofman commented Sep 13, 2024

Oh I never thought about this, is it not possible to proxy records and tuples at all?

If they have typeof 'object', you can proxy them, but such a proxy wouldn't be considered an immutable R/T.

@danieltroger
Copy link
Author

Oh I never thought about this, is it not possible to proxy records and tuples at all?

If they have typeof 'object', you can proxy them, but such a proxy wouldn't be considered an immutable R/T.

Ahh right. I just wonder how that would work with equality but I think it just doesn't.

There may be a possibility that could be acceptable to stakeholders:
(...)
* R/T containing functions would never be equal to another R/T, even if that R/T has its nested function wrapping the same original function.

Appreciate the idea but now my original usecase is broken.


Thank you all for the answers, I'll just go use a WeakMap with symbols if I ever really need to have a function in a record or tuple.

And I hope this proposal moves forward soon. Already today using it in form of the polyfill provides nice benefits IMO.

@demurgos
Copy link

demurgos commented Sep 13, 2024

The main reason why I am interested in R/T is to have "compound primitive" types which are better than string. This means that the value is the identity. The main power that it unlocks compared to passing object references is that object references need to be kept around to be matched by equality (Object.is) but R/T values can be built at multiple places and still be equal.

As an example use, let's imagine that I want to represent a map from (x,y) coordinate to some metadata like place name. Using an regular object, if I insert let coords = {x: 1, y: 3}; myMap.set(coords, "home") then I need to keep passing coords for myMap.get(coords) to work. With records, I can do myMap.set(#{x: 1, y: 3}, "home") and then I may create a new record value in an entirely different place and use myMap.get(#{x: y, y: 3}) to retrieve the value.

Having functions, or any boxed type, means that this property of being able to create similar records from scratch is no longer there. You'd have to keep reuse the same function reference, but if you already have to keep references around then I feel that it weakens the use-case: just keep a ref to the wrapper object instead. (There are still some cases where it's useful to only rebuild the wrapper object, but I feel like it's a less compelling use case).

@Maxdamantus
Copy link

Having functions, or any boxed type, means that this property of being able to create similar records from scratch is no longer there. You'd have to keep reuse the same function reference, but if you already have to keep references around then I feel that it weakens the use-case: just keep a ref to the wrapper object instead.

I don't really understand this argument. Why is a "ref to the wrapper object" (presumably a symbol stored in a WeakMap) better than the function value itself? It seems strictly worse to me (in terms of performance, code understandability, debugging, etc) and only exists as a workaround for not being able to directly store the desired value.

Function/object identity is a perfectly sensible concept (otherwise we would throw errors when people try to compare them or use them in Maps), and I think it makes sense to create "similar records from scratch" using object identities just as it makes sense using string/number values.

I'm also not aware of any language that has composite/struct/record/product values but which restricts the type of values that can be composited. Pretty much every modern language either has this feature already, or is moving in this direction.

@ljharb
Copy link
Member

ljharb commented Sep 14, 2024

@demurgos thats not a capability the language offers, or is likely to offer ever - you can make object wrappers but those simply aren’t primitives in any sense of the word.

@Maxdamantus
Copy link

#390 (comment)

There may be a possibility that could be acceptable to stakeholders:

* Functions when being used to create a record/tuple, would automatically be "wrapped" (similar to bound): a new frozen function object would instead be used in the R/T. That wrapper simply calls the original function.

* R/T containing functions would never be equal to another R/T, even if that R/T has its nested function wrapping the same original function.

* R/T must have a typeof `'object'`, as a primitive shouldn't enable a communication channel on its own

* There is a predicate to know if a R/T contains a nested function

* The ShadowRealm callable boundary could let these R/T through by replacing the function with a callable boundary wrapper.

This is sort of a variation of the "Box" idea, but without equality

Edit: The reason this may be acceptable is that an object containing a function is likely already considered today as not being pure.

I don't understand this reasoning. Being able to store a function is more-or-less equivalent to being able to store an object, since you can always wrap an object with a dummy function that returns it:

function wrap(v) {
    return () => v;
}

const obj = {};
const rec = #{ obj: wrap(obj) };
const obj_ = rec.obj();

There doesn't seem to be any difference in "purity" or in creating communication channels between your proposal above and the more obvious proposal of simply allowing objects/functions to be stored, so to me it seems like this proposal is both more complicated and also stifling the equality-based use cases for no reason.

Having special behaviour at ShadowRealm boundaries is perfectly sensible but I don't see why it would need wrapper functions or boxes.

I'm not familiar with the current ShadowRealm specification, but a user-level implementation of a membrane should be able to treat R/T values specially[0] so that if the R/T value contains an object, the object is replaced with an appropriate proxy (just as if that object were passed through the membrane itself). If the R/T value contains no object, it should be able to detect that easily (using the "predicate to know if a R/T contains a nested function object") as an optimization to avoid unnecessary allocation of duplicate R/T values. Sample code copied from a previous comment:

function adapt(v) {
    if (Object.isDeeplyImmutable(v)) // eg: 4, "foo", #[#{ bar: #["baz"] }]
        return v;
    if (Tuple.isTuple(v)) // eg: #["foo", {}]
        return v.map(adapt);
    if (Record.isRecord(v)) // eg: #{ foo: {}, bar: "baz" }
        return ...;
    // normal logic for proxying objects
    ....
}

This will also allow the original R/T value to be recovered if it's passed back through the membrane to the original realm (though it might be a reallocated version of the same value, like how someString.split("").join("") will probably be a reallocated version of the someString value).


[0] As a side note, just thought I'd mention that if the membrane doesn't treat R/T values specially and typeof #[] === "object", passing R/T values still more-or-less works as expected, the only difference is that cross-realm R/T values won't be detected as such, since Record.isRecord(recordFromOtherRealm) will be false, and they will compare inequal to any R/T value constructed in the non-originating realm. This is free (assuming R/T values can be used as WeakMap keys) and seems like fairly reasonable fallback behaviour for realm/membrane implementations that haven't yet been updated to handle R/T values.

@mhofman
Copy link
Member

mhofman commented Sep 25, 2024

Being able to store a function is more-or-less equivalent to being able to store an object, since you can always wrap an object with a dummy function that returns it

Sure, but the fact you have to call the function makes it explicit you're exiting the realm of immutable "static" data.

stifling the equality-based use cases for no reason

That is one of the problems with allowing mutable things in immutable structures. Their unforgeable identity matters and becomes part of the equality of the containing R/T. Some people believe that this should only happen with an explicit signal from the author.

This is particularly true for objects, which are easy to confuse with records, One could argue that a function is an explicit enough signal that the author intends to include mutable data in a R/T. However as I explained simply using the function object provided by the user is not acceptable as it can be used as a communication channel on its own, which is why I suggested automatically wrapping it. However that causes complications with equality: either you give up on equality matching by always creating new wrappers, or we effectively end up introducing a global (and possibly undeniable) WeakMap from original function to wrapped function in order to provide for stability.

I'm not familiar with the current ShadowRealm specification, but a user-level implementation of a membrane should be able to treat R/T values specially

The ShadowRealm proposal is structured in such a way that it's impossible to mix the object graphs of the 2 realms: the callable boundary only allows primitives and callables through, and the latter are automatically wrapped to enforce that only primitives and callables can pass through arguments and return values. It's effectively a small membrane that disallows objects to be shared between the 2 realms. As such you cannot create a proxy and pass it through the membrane as-is (there is a fancy membrane maintained by Salesforce that works with the limitations of the callable boundary, but it effectively relies on causing side effects on shared stack state)

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

6 participants