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

PrimeNG Angular Table "Converting circular structure to JSON" whenever table data contains non-trivial data #14428

Closed
rubmz opened this issue Dec 25, 2023 · 3 comments
Labels
Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible

Comments

@rubmz
Copy link

rubmz commented Dec 25, 2023

Describe the bug

Our application uses template components in its table. These components are given a reference to their own AbstractControl for various reasons. The problem is, that as result these components are being stored in the selected property of the PrimeNG table when user selects a row.
Then, when the user clicks on a row, Table.handleRowClick() is being called.
Table.handleRowClick() calls Table.saveState() which tries to JSON.stringify() on Table.selected.
JSON.stringify() doesn't like so much AbstractControls (and RXJS Subjects, which are also useful to pass to some table components) as they contain circular references internally. This raises an exception, and the state is not saved.

One could argue we should only store pure data in our table.data property, but it is many times important to pass more complex structures to the components. And with current implementation this breaks the handleRowClick() call.

A simple solution - pass a circular handler to JSON.stringify() as so:

const getCircularReplacer = () => {
  const seen = new WeakSet();
  return (key, value) => {
    if (typeof value === "object" && value !== null) {
      if (seen.has(value)) {
        return;
      }
      seen.add(value);
    }
    return value;
  };
};

saveState() {
  ...
  storage.setItem(this.stateKey, JSON.stringify(state, getCircularReplacer()));
}

Environment

Angular 16.1.7 in browsers web site.

Reproducer

No response

Angular version

16.1.7

PrimeNG version

16.1.0

Build / Runtime

TypeScript

Language

TypeScript

Node version (for AoT issues node --version)

18.18.2

Browser(s)

Chrome 120.0.6099.129

Steps to reproduce the behavior

  1. Create a table component and pass rows [data] a some template components that accepts their own (or other) controls (any Angular control) as part of their row data.
  2. Click on a row data.

The result -> exception in the console log.

Expected behavior

The state is saved without exception.

@rubmz rubmz added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Dec 25, 2023
@mbraeuner
Copy link

I had a similar issue with the tree table.
I had also nested data inside the tree table component. Every time I selected a tree node primeng threw an exception
screenshot primeng 2024-01-16 18-40-48

seems to be a recursive call of (https://github.com/primefaces/primeng/blob/239b86cdffe1ca4e8c0c03d13ad6b5ae8ad8cc3b/src/app/components/utils/objectutils.ts#L7)

Finally I removed the nested data references. But after seeing the error again on screenshot... Could be a workaround to implement a toString function for the data objects.

I'm wondering why primeng compares the whole object. A kind of trackby function could save performance while comparing selected copies with the original data

@rubmz
Copy link
Author

rubmz commented Jan 21, 2024

For starters they could use lodash.isEqual() which for the best of my knowledge do the work without the circular issue...

@mertsincan
Copy link
Member

Hi,

So sorry for the delayed response! Improvements have been made to many components recently, both in terms of performance and enhancement. Therefore, this improvement may have been developed in another issue ticket without realizing it. You can check this in the documentation. If there is no improvement on this, can you reopen the issue so we can include it in our roadmap?
Please don't forget to add your feedback as a comment after reopening the issue. These will be taken into account by us and will contribute to the development of this feature. Thanks a lot for your understanding!

Best Regards,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible
Projects
None yet
Development

No branches or pull requests

3 participants