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

Not able to store circular object in the node.data with ext-dnd5 #1053

Closed
stagefright5 opened this issue Nov 27, 2020 · 9 comments
Closed

Not able to store circular object in the node.data with ext-dnd5 #1053

stagefright5 opened this issue Nov 27, 2020 · 9 comments

Comments

@stagefright5
Copy link
Contributor

stagefright5 commented Nov 27, 2020

I want to store the node's ref inside the node.data object but using dnd5 extension, it throws error
image

image

Expected and Actual Behavior

Someway to allow circular object (may, safe stringify?)
or allow users to override the node.toDict

Steps to Reproduce the Problem

  1. store node itself inside the node.data
  2. drag the node

Environment

  • Browser type and version:
  • jQuery and jQuery UI versions:
  • Fancytree version: 2.37
    enabled/affected extensions: dnd5
@stagefright5
Copy link
Contributor Author

stagefright5 commented Nov 28, 2020

My current workaround is to override the every node's(in this case, only non-folder node's) toDict in the createNode hook like this:

const ftreeOptions = {
    createNode: (e, data) => {
        if (!data.node.isFolder()) {
            // Store the reference to original `toDict`
            const oDict = data.node.toDict;
            // "Override" `toDict` to skip the circular ref property (assign to new function)
            data.node.toDict = (...args) => {
                // Call the original `toDict` in `data.node`'s context and store the result in `d`
                const d = oDict.call(data.node, ...args);
                // `d.data.propObjKey` has the circular ref property, find if it exists
                if(Object.keys(d.data).find(k => k === 'propObjKey')) {
                       // It exists. So, create a new object without the
                       // the property that holds circular ref (ie., `d.data.propObjKey.__host_tree_node__`)
                       const newPropObj = {};
                       Object.keys(d.data.propObjKey).forEach(key => {
                            if (key === '__host_tree_node__') {
                                // THIS IS THE PROPERTY THAT HOLDS REFERENCE TO THE NODE IT IS RESIDING IN.
                                // THIS IS SOURCE OF CIRCULAR REF.
                                // SO, DO NOT CONSIDER IT, SKIP IT
                             } else {
                                 // Populate other key-value pairs
                                 newPropObj[sk] = d.data.propObjKey[sk];
                             }
                        });
                        // Finally, assign the new serializeble object to `d.data.propObjKey`
                        d.data.propObjKey = newPropObj ;
                }
                // Return serializeable object
                return d;
            };
        }
    }
}

@mar10
Copy link
Owner

mar10 commented Nov 29, 2020

Maybe an optional dnd5.copySource callback can be added.

@stagefright5
Copy link
Contributor Author

Maybe an optional dnd5.copySource callback can be added.

@mar10 ,
Yes and which will, of course, as with other callbacks, recieve the EventData object as defined here )

@stagefright5
Copy link
Contributor Author

stagefright5 commented Nov 30, 2020

@mar10 ,
This has to be changed in all the places where the node.data is being serialized. May be introduce a FancyTreeOptions.getSerializableData callback to get the serializable data of the node, which can be used by all the extensions too to get the node data. Or, toDict can call this method instead of directly accessing the node.data:
I mean instead of something like this: (line.no: 2333)
image

It could be like this: (line.no: 2333)
image

And tree options would be:
Something like this:

const ftreeOptions = {
    getSerializableData: (data) => {
        /*
          User defined logic to construct and return the
          serializable data goes here
        */
    }
}

@mar10 mar10 closed this as completed in 8f501f2 Nov 30, 2020
@mar10
Copy link
Owner

mar10 commented Nov 30, 2020

I don't want to encourage users to store recursive references in node.data, but I added an experimental new option dnd5.sourceCopyHook.
Here you can pass an optional callback that will passed to toDict() on dragStart.
This callback is the recommended way to serialize node structures anyway.

@mar10
Copy link
Owner

mar10 commented Nov 30, 2020

Could you test if this works for you?
I think you only need to patch this line in jquery.fancytree.dnd5.js:

var nodeData = node.toDict(true, dndOpts.sourceCopyHook);

and then pass the new option

dnd5: {
    ...
    sourceCopyHook: function(dict, node) {
        ...
    }
}

@stagefright5
Copy link
Contributor Author

I don't want to encourage users to store recursive references in node.data

why not?

I will test it and let you know.

@stagefright5
Copy link
Contributor Author

@mar10 ,
It seems to be working in my case. Thanks a ton!

@mar10
Copy link
Owner

mar10 commented Dec 1, 2020

why not?

Because the data property is typically filled automatically from JSON data on init or when lazy-loading (i.e. all unknown JSON attributes go into node.data).

In these cases you would not have cyclic structures or references either.
These kind of references are typically created after loading by custom code (kind of compiling/caching). I think it would be cleaner to store this in a different place, e.g. node._foo or node.cache._foo.
And keep the node.data to store serializable information that allows to create a complete node.

Thanks for testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants