Add option preserveReferences

[remcohaszing]

With this option, all references within the object will be preserved. This also allows to serialize recursive structures. If needed, the resulting expression will be an iife.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (e3e7e78) to head (1432fb7).

Additional details and impacted files

@@            Coverage Diff             @@
##              main        #3    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            1         1            
  Lines          218       634   +416     
  Branches        49       123    +74     
==========================================
+ Hits           218       634   +416     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[wooorm]

It’s a ton of code, so I don’t grasp it all. But some Qs:

• not sure about the word “reference”; it’s not wrong per se; more, is that the word users will expect when they are thinking about this?
• should whether to support cyclical things be different from whether to support references?
• should the default be the other way around? this library has so much code to e.g. detect array buffers and whatnot and recreate them faithfully; say, if someone has a modified and published to the same date object, I think it makes sense that that too would faithfully be references to one object; and then perhaps the option can be flipped: thinking out loud, maybe dereference: boolean (default: false)?
• not sure what to do with the IIFE though, I don‘t think there’s a way around it, and it’s not very pretty for default behavior

[remcohaszing]

It’s a ton of code, so I don’t grasp it all. But some Qs:

❤️

• not sure about the word “reference”; it’s not wrong per se; more, is that the word users will expect when they are thinking about this?

I like the word “reference”, but I’m open to alternatives.

• should whether to support cyclical things be different from whether to support references?

Personally I don’t think so.

• should the default be the other way around? this library has so much code to e.g. detect array buffers and whatnot and recreate them faithfully; say, if someone has a modified and published to the same date object, I think it makes sense that that too would faithfully be references to one object; and then perhaps the option can be flipped: thinking out loud, maybe dereference: boolean (default: false)?

The modified and published date refer to the same date object with the changes as-is in this PR. :)

I think it makes sense to flip the default behaviour, and I like the name dereference. The main reason to add the option at all IMO, is because

1. The current result is relatively easy to inverse (Question. Does there is opposite to this library like ESTree -> JSON #2)
2. Backwards compatibility. This is why I decided to make the old behaviour default.

• not sure what to do with the IIFE though, I don‘t think there’s a way around it, and it’s not very pretty for default behavior

I think this is the only way. I don’t think it’s pretty, but practically I think it doesn’t really matter for most use cases.

[wooorm]

The modified and published date refer to the same date object with the changes as-is in this PR. :)

Huh? I thought the docs said the inverse? `preserveReferences` (boolean, default: `false`)? When I read that I’d assume that, if modified and published refer to the same date object on input, then the output would be two different date objects. That as a user I need to pass preserveReferences: true to make them reference one date object.

---

GH-2 is asking about sval I think? Although maybe a very limited version?

I think we talked about this in DMs but mentioning it here to be sure: I’ve investigated how to do that too, but safely, and it’s not possible except when you stick to math (1 + 2 // 3)

---

I think this is the only way. I don’t think it’s pretty, but practically I think it doesn’t really matter for most use cases.

Maybe https://github.com/estree/estree/blob/master/experimental/do-expressions.md?

[remcohaszing]

That as a user I need to pass preserveReferences: true to make them reference one date object.

Yes, this is correct. I was confused because you singled out Date objects here.

GH-2 is asking about sval I think? Although maybe a very limited version?

I think we talked about this in DMs but mentioning it here to be sure: I’ve investigated how to do that too, but safely, and it’s not possible except when you stick to math (1 + 2 // 3)

Hmm yes, I think sval is a good recommendation for that use case. I didn’t know about sval at the time that issue was reported.

Maybe https://github.com/estree/estree/blob/master/experimental/do-expressions.md?

Absolutely! But only if https://github.com/tc39/proposal-do-expressions reached stage 4 and is widely supported.

[wooorm]

lgtm btw, I don’t have blockers