On 04/04/2024 12:59, Thomas Lamprecht wrote: > Am 04/04/2024 um 12:10 schrieb Friedrich Weber: >> Maybe we could do: >> >> ```js >> extraRequestParams: {}, >> >> constructor: function(conf) { >> let me = this; >> me.extraRequestParams = Ext.clone(me.extraRequestParams); >> me.initConfig(conf); >> me.callParent(); >> }, >> ``` >> >> ... which, if I'm not missing anything, *should* cover everything (with >> the cost of allocating unnecessary empty objects)? >> > > > yeah, I just wanted to suggest something like that, albeit I first had the > (a few times used): > > me.extraRequestParams = Ext.apply({}, me.extraRequestParams ?? {}); > > pattern in mind. Mostly an slight performance improvement as we can assume > that this is either undefined or an object, while Ext.clone checks for all > different types (in a legacy browser compat way). > > Albeit nowadays one might be even better off to use something like the > spread operator: > > me.extraRequestParams = { ...(me.extraRequestParams ?? {}) }; > > https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#spread_in_object_literals > > Or maybe even nicer, the Object.assign operator, that does not throw if > the sources are null or undefined: > > me.extraRequestParams = Object.assign({}, me.extraRequestParams); > > https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign > > But all those are implementation details (of which I probably would prefer > the Object.assign one the most, albeit no hard feelings), in general your > proposed solution seems to be the best one, w.r.t sane new usage and > backward compat.
I agree `Object.assign` looks like the nicest solution, and seeing that we already use it in our codebase, I'll go for this one. > btw. hasn't the `submitOptions` config object the same issue? You're right, `submitOptions` also has the potential for the same class of bugs. The current usages of `submitOptions` look unproblematic. But since we go for a more general fix for `extraRequestParams` anyway, let's handle `submitOptions` similarly. I'll send a v3 (just noticed I forgot the `v2` in the subject for this one). Thank you and sterzy for your suggestions! _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel