HaxeFoundation / haxe

Haxe - The Cross-Platform Toolkit
https://haxe.org
6.03k stars 648 forks source link

Reflect.hasField returns false on property getters on the JavaScript target #11613

Open EliteMasterEric opened 3 months ago

EliteMasterEric commented 3 months ago

Reproduction

https://try.haxe.org/#8C3c6C77

Explanation

I have encountered some strange behavior with regards to Reflection against non-physical properties on the JavaScript target.

I am working on a fork of HScript, which needs to function properly on the HTML5 target. As part of variable resolution, I use Reflect.hasField to determine if I should resolve a field or throw a syntax error.

On the Eval target, Reflect.hasField('myValue') returns false, but Reflect.hasField('get_myValue') returns true. This allows me to determine if getProperty (which the docs state is slower than Reflect.field) is necessary.

However, Reflect.hasField('get_myValue') returns false on JavaScript, even though Reflect.getProperty('myValue') does succeed. This gives me no way to detect if getProperty will return a valid result or not (checking getProperty() == null won't work since there is no way to distinguish between "this value exists and is null" and "this value does not exist").

I cannot use Reflect.properties() or Reflect.hasProperty(), because no such functions do not exist for some reason.

Additional notes:

back2dos commented 3 months ago

On the JavaScript target, enabling full DCE actually causes getProperty('myField') to fail, presumably because the getter function is getting culled. Getter and setter functions should definitely never get culled if the class itself has not been culled.

DCE is granular at field level by design.

If anything, the argument could be made that Reflect.getProperty should keep all properties on kept classes.

If you can, avoid reflection like the plague. More often than not, safer and faster alternatives are available. If not, you could either use addGlobalMetaData to whatever packages you wish with to perform reflection on, or add an onGenerate hook that marks all getters and setters with @:keep.

Additionally, there could be a Reflect.properties and Reflect.hasProperty. Made a JS implementation here: https://try.haxe.org/#5611215a

That said, I would yet again advise against using such a thing though ;)