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

fix: decycle potentially cyclic structures before serializing #634

Merged
merged 9 commits into from
Jul 22, 2021

Conversation

sarahdayan
Copy link
Member

@sarahdayan sarahdayan commented Jul 20, 2021

This introduces a decycle utility to replace cyclic references in objects before serializing them with JSON.stringify.

It uses a slightly modified version of Crockford's cycle.js, which MDN recommends. This works better than JSON.stringify's replacer, because the latter walks every node in the object, making it difficult to preserve non-circular structures like the following:

const a = { a: 1 };
const obj = [a, a]; // JSON.strinigfy can handle this, we don't want to end up with `[{ "a": 1 }, "[Circular]"]`

Whenever a circular reference is met, it's replaced with [Circular] (à la Jest).

{
  "self": "[Circular]"
}

If sources contain complex objects (e.g., VNodes with SVGs), decycling could have a negative impact on performance. I've therefore introduced a change to invariant, making it accept message in the form of a function to execute only when the condition isn't met.

fixes #618

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 20, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 79dacdd:

Sandbox Source
@algolia/autocomplete-example-github-repositories-custom-plugin Configuration
@algolia/autocomplete-example-instantsearch Configuration
@algolia/autocomplete-example-playground Configuration
@algolia/autocomplete-example-preview-panel-in-modal Configuration
@algolia/autocomplete-example-react-renderer Configuration
@algolia/autocomplete-example-starter-algolia Configuration
@algolia/autocomplete-example-starter Configuration
@algolia/autocomplete-example-voice-search Configuration
@algolia/autocomplete-example-vue Configuration
friendly-lamarr-y7tur Issue #618

@sarahdayan sarahdayan changed the base branch from next to build/sass-implementation July 20, 2021 23:49
@sarahdayan sarahdayan marked this pull request as ready for review July 20, 2021 23:56
Base automatically changed from build/sass-implementation to next July 21, 2021 10:05
Copy link
Member

@francoischalifour francoischalifour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice change–we could definitely reuse these dev warnings in our other frontend libraries (and abstract them more).

packages/autocomplete-shared/src/invariant.ts Outdated Show resolved Hide resolved
packages/autocomplete-shared/src/decycle.ts Show resolved Hide resolved
packages/autocomplete-shared/src/invariant.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Co-authored-by: François Chalifour <francoischalifour@users.noreply.github.com>
@sarahdayan
Copy link
Member Author

we could definitely reuse these dev warnings in our other frontend libraries (and abstract them more)

I've added a ticket for this in the backlog. It could be an onboarding task if we don't get to do it before.

@sarahdayan sarahdayan merged commit 99f7c84 into next Jul 22, 2021
@sarahdayan sarahdayan deleted the fix/cyclic-references branch July 22, 2021 10:04
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.

Cyclic references because of VNode serialization
3 participants