Core: Switch to Object.defineProperty( this, 'is*', { value: true } );

[RenaudRohlinger]

Tried to give a try to the this.is* commit, everything looks ok so far except when serializing / deserializing TypeError: Cannot add property isVector3, object is not extensible

I had to convert all the math class to this pattern:
Object.defineProperty(` this, 'isVector3', { value: true } );

Related:
#24047 #24092

[Mugen87]

except when serializing / deserializing

Um, I don't see any problems with the editor. How can the error be reproduced?

[Mugen87]

I had to convert all the math class to this pattern: Object.defineProperty(` this, 'isVector3', { value: true } );

Probably no good idea: #21284.

[RenaudRohlinger]

Ah I see, indeed. that was when sending vec3 via postMessage with service workers. I will try to investigate on that end then. (I will just use toArray, fromArray I guess ^^)

Should we close this issue then?

[marcofugaro]

Probably no good idea: #21284.

Yup, we used Object.defineProperty() in the past but turns out it was slow.

[Mugen87]

Should we close this issue then?

Yes. I don't think this issue is a reason for reverting the PR.

[Mugen87]

@marcofugaro I've just noticed that public instance fields are internally added via Object.defineProperty(), see here.

Public instance fields are added with Object.defineProperty() either at construction time in the base class (before the constructor body runs), or just after super() returns in a subclass.

That means public instance fields could eventually introduce the same performance issue like when directly using Object.defineProperty(). If we convert .is* flags to public instance fields in the future, we should definitely have an eye on the performance. And probably report any performance issues to browser vendors.

[marcofugaro]

@Mugen87 ah yes good point, we need to do performance profiling before switching to class fields. And report eventual finding as you said.