Discord.js: [Feature request] Provide a convenient way to deep copy RichEmbeds

[thiemok]

It would be nice if there was a convenient way to obtain a deep copy of a RichEmbed instance.

This could be achieved by adding a clone() method to RichEmbed or even simpler by changing RichEmbeds constructor to deep copy its data parameter before assigning it to properties, which should probably be done anyways.

[Drahcirius]

May I ask what you want to deep copy a RichEmbed for? If it's for editing and re-sending embeds, that's already possible on latest master.

[thiemok]

Some of my messages can exceed the limits of a single embed. So i might need to build more than one embed, while some properties like author and thumbnail have the same values in all embed build to fit that messages content. I would like to configure all the shared parts of an embed and then copy it to add all the content that needs to be split across multiple embeds.
While this is certainly achieveable with v11.1, its inconvenient to do (havent found any changes regarding this on master, but correct me if im wrong)

Currently it requires me to either perform a object copy before suppling the existing RichEmbed to the RichEmbed constructor, which just seems wrong to me and does not work well with flow

//@flow
const embed1 = new RichEmbed();
// configure embed...
const embed2 = new RichEmbed(Object.assign({}, embed1: Object));

Or build the shared embed part directly as a simple object, which defeats the purpose of using RichEmbed in the first place and breaks as soon as the shared part is accidentially modifed somewhere.

const bar = {
    author: {
        name: 'foo',
        url: 'https://example.com',
    },
}
const embed2 = new RichEmbed(bar);

This is probably a simple addition to the code and i'd happily supply a pr myself if this is a desired addition/ change to the constructor of RichEmbed.

[devsnek]

just don't send enormous amounts of data. smh you can upload information to gist or something. your current method seems fine as it is.

[thiemok]

Lets not narrow this down on my current use case.
What im trying to say is, that RichEmbeds constructor has side effects on the object supplied as the data parameter. This also defeats other use cases like templating embeds or allows to change embed contents without limit checks. I dont think it should have this behaviour, but it might be intentional so any other way to deep copy the instance would also be fine.

[JohnDoesCodes]

hmmmmm

[Drahcirius]

I see what you're saying, it's unfortunate that the only reason master doesn't already support this is because fields don't get deep copied. Otherwise it can already be done as follows:

const origin = new MessageEmbed().setTitle('hi').setColor(0xCCCCCC).setAuthor(msg.member.displayName);
const foo = new MessageEmbed(origin).setDescription("hello");
const bar = new MessageEmbed(origin).setDescription("world");
msg.channel.send(foo);
msg.channel.send(bar);

So if you're not using fields, this would work. I can look into getting this functionality working for fields later today.

[thiemok]

That would be great. For the sake of completeness:
Should things like this be possible, or also be changed?

const bar = {
    author: {
        name: 'bar',
        url: 'https://example.com',
    },
}
const embed = new RichEmbed(bar);

bar.author.name = 'foo';
console.log(embed); // embed.author.name is now === 'foo'

[Drahcirius]

The original object shouldn't alter the new embed because it would need to be deep copied when passed into the constructor in order to facilitate the kind of "parent" embed functionality you're looking for. However, you would be able to do it if you replaced bar.author.name with embed.author.name as if it was a simple object.

[thiemok]

Yes, i think we mean the same thing. That was just an example of how an embed constructed with the current code would behave. But should it stay that way?

[Drahcirius]

It does not behave that way on latest master (v12) because author gets deep copied. Fields is the only reason it does not work as you hoped it to be.
I'll also look to see if I can backport some of these for 11.2 later on.

[thiemok]

Oh well, excellent case of looking for something and not seeing it, even if its clearly there😅

[thiemok]

Since my blindness already wasted enough of your time, let me suggest the fix for fields

    /**
     * The fields of this embed
     * @type {Object[]}
     * @property {string} name The name of this field
     * @property {string} value The value of this field
     * @property {boolean} inline If this field will be displayed inline
     */
    this.fields = data.fields ?
        Array.from(data.fields, field => Object.assign({}, field))
        : [];

[Drahcirius]

I had intended to backport #1859 so that you would use .toJSON instead of making a new .clone method and keep forward compatability, but 11.2 was released today, so unfortunately you'll have to clone it manually or use master when #1864 gets merged.

[thiemok]

Thanks, i already added a crude fix to my code and will switch to a more elegant solution once the current master gets merged to stable.

[devsnek]

toJSON and clone are not the same thing and shouldn't be treated as such

[iCrawl]

This is implemented in latest master as Drah said, no backports are planned.