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

Feature request: named circular references #43

Open
valerii15298 opened this issue May 13, 2023 · 5 comments
Open

Feature request: named circular references #43

valerii15298 opened this issue May 13, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@valerii15298
Copy link

valerii15298 commented May 13, 2023

When using util.inspect(myObject)) circular references are named like ref 1, ref 2 etc...
Would be great to have an option for named circular references, something like this:

const stringify = configure({namedCircularReferences: true});
const circular = {a: 1};
circular.circular = circular;
stringify(circular);
// {__ref: "Ref 1", circular: "Ref 1"
@BridgeAR
Copy link
Owner

The algorithm works a bit different than in util.inspect(). A big reason is that the intent here is adhere to always output a format that is possible to be parsed with JSON.parse(). It is possible to add named references similar to:

const circular = { a: 1, outer: { prop: {} } }
circular.outer.prop.circular = circular

console.log(stringify(circular))

{ "a": 1, "outer": { "prop": { "circular": "[Circular: ~.outer.prop.circular.~]" } } }

delete circular.outer.prop.circular
circular.outer.prop.circular = circular.outer.prop

console.log(stringify(circular))

{ "a": 1, "outer": { "prop": { "circular": "[Circular: outer.prop~.circular.~]" } } }

The current algorithm is mainly tuned for performance and adding named references would have another tracking overhead. It requires another array to keep the names. I did not check how bad the overhead would be, but that has to be checked first.

@valerii15298
Copy link
Author

valerii15298 commented May 14, 2023

@BridgeAR thanks for the response. Your idea with path as strong in circular reference is even better! One of the main benefits as I see it: we will not lose data about circular references and it will be serializable/deserializable.

About performance: that is why I suggest to add such functionality via flag in configure. So that by default it will be turned off and thus will not impact main performance metrics I guess?

@valerii15298
Copy link
Author

valerii15298 commented May 14, 2023

Btw, I would imagine it to work something like that:

const circular = {
    a: 1,
    outer: {
        prop: {}
    }
}
circular.outer.prop.circular = circular.outer

console.log(stringify(circular))

{
  "a": 1,
  "outer": {
    "prop": {
      "circular": "[Circular: &.outer]"
    }
  }
}

On my opinion easier to understand(& is like reference to original object)

@BridgeAR
Copy link
Owner

BridgeAR commented Jun 8, 2023

I have a reference implementation but it does slow down the overall serialization.
Using an option is not really helpful, due to having multiple if branches to check if the option is active and they are always hit.

@valerii15298
Copy link
Author

I'm not an expert in performance but doesn't such approach work?
Instead of:

if (flagName) {
  // do something
} else {
  // do something else
}

use

const actionsConfig = {
  featureName: () => {
    // do something
  },
  withoutFeatureName: () => {
    // do something else
  },
}

const flagName = 'featureName'
actionsConfig[flagName]();

The point is that second approach should be O(1) complexity.
Not sure if something like that could help bcs it is idea without knowing anything about implementation.
Also I don't know how costly is property access in js so there could be tradeoffs.
In general I got you, and thanks for paying attention to it. Maybe at some point it will be possible to implement solution without performance loss.
For now this feature is not high priority on my opinion, though useful sometimes.

@BridgeAR BridgeAR added the enhancement New feature or request label Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants