Skip to content

Conversation

@maoberlehner
Copy link
Contributor

Instead of manually setting a flag to stop enriching, we now check if an object has already been enriched. This ensures that all linked objects are enriched, even in cases of circular dependencies.

A WeakSet is used to track enriched objects, ensuring they can be garbage collected when no longer referenced.

BREAKING CHANGE: The client's output is no longer guaranteed to be serializable (e.g., with JSON.stringify). Due to the full resolution of circular dependencies, the resulting object graph may contain cycles.

Fixes WDX-146

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes circular link dependency handling in the JavaScript client by replacing manual flag-based cycle prevention with a WeakSet-based tracking system. This ensures all linked objects are enriched while properly handling circular references.

  • Replaced _stopResolving flag mechanism with a WeakSet to track enriched objects
  • Removed manual object copying (_cleanCopy) to allow proper circular reference handling
  • Updated tree traversal to use WeakSet for cycle detection instead of path tracking

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Instead of manually setting a flag to stop enriching, we now check if an
object has already been enriched. This ensures that all linked objects
are enriched, even in cases of circular dependencies.

A `WeakSet` is used to track enriched objects, ensuring they can be
garbage collected when no longer referenced.

BREAKING CHANGE: The client's output is no longer guaranteed to be
serializable (e.g., with `JSON.stringify`). Due to the full resolution
of circular dependencies, the resulting object graph may contain cycles.

Fixes WDX-146
@maoberlehner maoberlehner force-pushed the bugfix/js-client-enriching-circular-links branch from 7399762 to 3b02402 Compare September 10, 2025 06:58
@pkg-pr-new
Copy link

pkg-pr-new bot commented Sep 10, 2025

Open in StackBlitz

@storyblok/astro

npm i https://pkg.pr.new/@storyblok/astro@301

storyblok

npm i https://pkg.pr.new/storyblok@301

@storyblok/eslint-config

npm i https://pkg.pr.new/@storyblok/eslint-config@301

@storyblok/js

npm i https://pkg.pr.new/@storyblok/js@301

storyblok-js-client

npm i https://pkg.pr.new/storyblok-js-client@301

@storyblok/management-api-client

npm i https://pkg.pr.new/@storyblok/management-api-client@301

@storyblok/nuxt

npm i https://pkg.pr.new/@storyblok/nuxt@301

@storyblok/react

npm i https://pkg.pr.new/@storyblok/react@301

@storyblok/region-helper

npm i https://pkg.pr.new/@storyblok/region-helper@301

@storyblok/richtext

npm i https://pkg.pr.new/@storyblok/richtext@301

@storyblok/svelte

npm i https://pkg.pr.new/@storyblok/svelte@301

@storyblok/vue

npm i https://pkg.pr.new/@storyblok/vue@301

commit: 3b02402

@imranolas
Copy link
Contributor

imranolas commented Sep 10, 2025

I do like the solution but given that it is a breaking change it might be worth holding off until we're comfortable bumping all the dependants too

@maoberlehner maoberlehner marked this pull request as draft September 11, 2025 10:40
@maoberlehner
Copy link
Contributor Author

Switched to draft mode because we have to further investigate if this is a change we want to do. Postponing the decision to the new content API client.

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.

2 participants