fix(): Avoid errors on restoring custom properties that pass the lazy detection of shadow,gradient,pattern and clipPath

[zhe-he]

When importing, each property of the object will be iterated over. If a property has type and source, additional logic will be executed. If the user has custom-exported additional properties, and the property is an object with type or source keywords, the user needs to output it as is.

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

[asturur]

Ok i see your point, but we can't return value on a catch, it will make unparsed pattern returned as is with errors later.
We need make stronger the check. i ll try to make some examples

[asturur]

Maybe we can change:

const promises = Object.values(serializedObject).map((value: any) => {

to

const promises = Object.entries(serializedObject).map(([key, value]: any[]) => {

and then we can use if ['gradient', 'shadow', 'clipPath'].includes(key) && value.type

that will make custom properties safe.

And you can do the same for patterns?

[zhe-he]

done.

[asturur]

Adding a test and merging

[asturur]

we forgot 3 cases.

[asturur]

I m having a second thought we did it the wrong fix here.

[asturur]

@zhe-he look at how i changed it, and let me know.

[zhe-he]

@asturur
The previous commit modifications are clear to me. I noticed that the last commit marked as 'ok' was reverted, and I don't understand whether it was due to our changes causing other bugs or if it was due to errors from a merge conflict.

[asturur]

The fix we made with the properties was making impossible extend a class with a restorable property. because if you added a custom property like 'mask' with a restorable object, then the allow list we built would have blocked you

[zhe-he]

Using hasClass is not very reliable either. I might have a custom attribute whose type value happens to exist in Fabric's types, such as Rect, Textbox, Path, etc. Can we first check if the key is a Fabric property, and then check if the value exists in the classRegistry?

[asturur]

That would be bad design on implementer side.
In fabric an object with type: rect is a rect. period.
We can't block everyone from adding rects to custom properties because someone could use objects with 'type: rect' that are not rects.

Why shoud you block someone from adding a custom property that is correctly enlivable?

[zhe-he]

Okay, if this is reasonable, then it is indeed unnecessary to over-design. I agree with you that allowing users to customize properties of built-in objects is a good idea.