Add default pack_models

[bollwyvl]

References

• naive, untested sketch of proposal from Error with 8.0.5 when selecting ipytree node #3735 (comment)

Changes

• hoists critical IPY_MODEL_ magic string to a constant
• removes some anonymous function definitions in (recursive) loops in unpack_models
• adds pack_models
  • uses for any unset-serializer that's a neighbor of deserialize: unpack_models
• makes use of the frozen JSONExt.emptyObject in some places to avoid allocating new objects

Questions

• does it need to be a promise?
• throw a warning?
  • ick. these get called a lot. maybe call once per class?
• hoist widget_serialization to mirror the backend API?

[github-actions]

👈 Launch a binder notebook on branch bollwyvl/ipywidgets/gh-3735-default-serializer

[bollwyvl]

This does appear to fix at least ipytree and the jslink condition:

[bollwyvl]

I've taken this out of draft and updated the notes a bit: probably don't have time to dig into the packaging file... more node, more problems.

[bollwyvl]

I've removed these here, and opened this issue: jupyter-widgets/team-compass#20

[maartenbreddels]

@bollwyvl thank you for taking a look at this!

I wonder if instead of having a pack_models implementation, we should use the "old" JSON.parse(JSON.stringify(x))) method instead. We may be breaking code that relies on the toJSON option, what do you think of that?

I'll see if I can at least add a test to make this not happen again.

[maartenbreddels]

In code, what I propose:

-const deepcopy =
-  globalThis.structuredClone || ((x: any) => JSON.parse(JSON.stringify(x)));
+const deepcopyJSON = (x: JSONValue) => JSON.parse(JSON.stringify(x));
+
+const deepcopy = globalThis.structuredClone || deepcopyJSON;

@@ -572,7 +574,7 @@ export class WidgetModel extends Backbone.Model {
 
         if (serialize == null && keySerializers.deserialize === unpack_models) {
           // handle https://github.com/jupyter-widgets/ipywidgets/issues/3735
-          serialize = pack_models;
+          serialize = deepcopyJSON;

I do like you pack_models and I think we should ship it, and use it, so we don't rely on the JSON trick in the ipywidget code base. This asymmetry has confused me many many times.

[bollwyvl]

I can't push at the moment, due to github issues (last tried a few minutes ago) but i'm up for anything that reduces active breakage in many downstreams. Feel free to continue/fork this however you can!

[maartenbreddels]

meeseeksdev please backport to 7.x

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

git checkout 7.x
git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -x -m1 022b4541c0cf848472993be48f13830ca71bfcac

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #3738: Add default `pack_models`'

4. Push to a named branch:

git push YOURFORK 7.x:auto-backport-of-pr-3738-on-7.x

5. Create a PR against branch 7.x, I would have named this PR:

"Backport PR #3738 on branch 7.x (Add default pack_models)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

[vidartf]

This URL seems to be broken? Maybe due to a rebase?