Refactor: Lazy Loading and Per-Type Subclasses for oNode Performance

[Tilix4]

Two optimizations:

1. Lazy Attribute Loading

• Attributes are no longer loaded in the constructor
• attributesBuildCache() and setAttrGetterSetter() run on first access to .attributes or shorthand properties

2. Per-Type Subclass Factory with Dynamic Inheritance

• Introduced oNodeTypes factory that creates and caches per-node-type subclasses
• On first use of a type, the factory:
  • Creates a temporary node to scan attributes via node.getAttrList()
  • Defines prototype shorthand getters (e.g., position, offset) that trigger lazy loading
  • Caches the subclass for reuse
• The oNode constructor delegates to the appropriate per-type subclass
• Named subclasses (oDrawingNode, oPegNode, etc.) inherit shorthand getters via _ensurePrototypeGetters()

3. Optimized getNodesByType()

• Replaced recursive subNodes() traversal with native node.getNodes([typeName]) API
• Filters paths by group scope, creating wrappers only for matching nodes
• Reduces wrapper object creation and traversal depth

Key Changes

• oNode constructor: Removed eager refreshAttributes() call; added lazy loading flags
• oNode.prototype.attributes getter: Triggers attributesBuildCache() and setAttrGetterSetter() on first access
• oNodeTypes factory: New system for creating per-type subclasses with prototype shorthand getters
• oGroupNode.prototype.getNodesByType(): Uses native Harmony API instead of recursive traversal
• Temporary scan nodes: Created with unique timestamped names and cleaned up in finally blocks

Benefits

• Faster node wrapping: Construction is O(1) instead of O(n) attributes
• Shorthand access preserved: node.position.x = 5 works immediately via prototype getters
• Backward compatible: All existing APIs work unchanged
• Efficient type scanning: Each node type is scanned once and cached

Testing

• Unit tests verify lazy loading behavior and shorthand attribute access
• Tests confirm temporary scan nodes are properly cleaned up
• Tests validate shorthand getters work across different node types (READ, PEG, etc.)

[mchaptel]

Lots of interesting things, but I do worry about some of the patterns and the compatibility with some edge cases.

I'm not sure if you know, but because of the cyclic nature of the access to the $ object embedded in each prototype (even though the $ object is created last), which was setup to fix the issue of lack of access of the global scope in some scenarios, we do this thing in base.js where we create the $ link on all the classes that were loaded from the lib. I think because of this it would be better to make oNodeType a class with a prototype and intitialize it during load instead of creating a simple dictionary type struct. It would also ensure that this.$ is properly accessible in the methods and not pointing to the global scope

Also in terms of philosophy of design I'm not sure about the return undefined that we have here, they could hide a lot of errors until later... I would rather we throw the errors when we reach them; also would be more representative for the testing.

And last thing, I think the current getter setter logic is flawed as it returns the attribute instead of getting/setting its value... Maybe there's a way to reuse the old logic here to avoid code duplication and inconsistent behavior?

I'm also not a fan of having to create dummy nodes to read the attributes. We can assume that oNode objects will be created as a wrapper around an existing node so we can simply use that I think?

[mchaptel]

Don't we have to add this on the oNode prototype instead? I think the jerry rigging of attaching $ to the object will only work if this is an instance method (otherwise you will lose access to this.$ inside the function body)

[mchaptel]

I think also this might break the attaching of $ to the prototype, which might make it an issue when the code is run from a package, dialog or Qtimer. I would make this a proper "class" (as proper as Qtscript has anyway) and intitialize it like a singleton during the lib init, then access it with this.$ inside the oNode constructor.

This will also make it stylistically closer to the other classes of the lib and ensure we control what 'this' points to in these methods.

[mchaptel]

Maybe that can just be a property like oNodeTypes.types that returns the object with keys as types and values as constructors? Then it will be easy for people to enumerate over them?

[mchaptel]

this is just a stylistic nitpick but I'm not a big fan of contracted/abreviated variable names for readability. I think maybe we could spell it constructor here to make it obvious?

But also, I'm thinking that there is something about the architecture that we can improve so we don't have to create the node. Why don't we pass the node path instead of type? Typically, oNodes are created to reflect existing nodes so it's redundant to create one for the type when we are currently wrapping an existing one. I agree it's less cleanly separated but asking the app to do something like adding a node has significant overhead compared to just running some js

[mchaptel]

not super clear here why we need the substring 50 ?

[mchaptel]

again here if attributes is null we have an issue, throwing an error would be better

[mchaptel]

getters setters are never full fledged attributes. They get set differently, and accept different values. Here we should return the value not the attribute I think

[mchaptel]

again, here I would prefer if we throw an error

[mchaptel]

should it not be

var desc = Object.getOwnPropertyDescriptor(attrs, kw);

here?

I'm a little confused about why we're not just accessing the attribute from the attribute object using the kw.

[mchaptel]

yeah I would really prefer not having to do this. Also not sure deleteNode needs a try catch?