ObjectLoader: Fix TS definition of .load()

[jeffreylanters]

The Object3D typings we're missing the material.

[Mugen87]

This seems to be a misunderstanding. Object3D does not have a material property.

[jeffreylanters]

This seems to be a misunderstanding. Object3D does not have a material property.

But it does work, when I log an Object3D object, I'll see and can set the material.

[Mugen87]

It looks like you are working with an instance of Mesh.

[jeffreylanters]

The variable comes straight out of the object loader, it's untouched. There might my another mistyping?

[jeffreylanters]

When I log the Object3D

The material does exist

[Mugen87]

Mesh is derived from Object3D. I think if you study the source code of Object3D and Mesh, things should become more clear:

three.js/src/core/Object3D.js

Lines 21 to 101 in 09e1fde

	function Object3D() {
	Object.defineProperty( this, 'id', { value: object3DId ++ } );
	this.uuid = _Math.generateUUID();
	this.name = '';
	this.type = 'Object3D';
	this.parent = null;
	this.children = [];
	this.up = Object3D.DefaultUp.clone();
	var position = new Vector3();
	var rotation = new Euler();
	var quaternion = new Quaternion();
	var scale = new Vector3( 1, 1, 1 );
	function onRotationChange() {
	quaternion.setFromEuler( rotation, false );
	}
	function onQuaternionChange() {
	rotation.setFromQuaternion( quaternion, undefined, false );
	}
	rotation.onChange( onRotationChange );
	quaternion.onChange( onQuaternionChange );
	Object.defineProperties( this, {
	position: {
	configurable: true,
	enumerable: true,
	value: position
	},
	rotation: {
	configurable: true,
	enumerable: true,
	value: rotation
	},
	quaternion: {
	configurable: true,
	enumerable: true,
	value: quaternion
	},
	scale: {
	configurable: true,
	enumerable: true,
	value: scale
	},
	modelViewMatrix: {
	value: new Matrix4()
	},
	normalMatrix: {
	value: new Matrix3()
	}
	} );
	this.matrix = new Matrix4();
	this.matrixWorld = new Matrix4();
	this.matrixAutoUpdate = Object3D.DefaultMatrixAutoUpdate;
	this.matrixWorldNeedsUpdate = false;
	this.layers = new Layers();
	this.visible = true;
	this.castShadow = false;
	this.receiveShadow = false;
	this.frustumCulled = true;
	this.renderOrder = 0;
	this.userData = {};
	}

three.js/src/objects/Mesh.js

Lines 20 to 33 in 09e1fde

	function Mesh( geometry, material ) {
	Object3D.call( this );
	this.type = 'Mesh';
	this.geometry = geometry !== undefined ? geometry : new BufferGeometry();
	this.material = material !== undefined ? material : new MeshBasicMaterial( { color: Math.random() * 0xffffff } );
	this.drawMode = TrianglesDrawMode;
	this.updateMorphTargets();
	}

[jeffreylanters]

Okay, I see what you mean. But still, the ObjectLoader packed in the module, returns an object with the Typescript Type Object3D, and it does contain material, but it's not typed.

[Mugen87]

returns an object with the Typescript Type Object3D, and it does contain material, but it's not typed.

Then there is a problem in ObjectLoader.d.ts. The onLoad() callback of ObjectLoader.load() can return many different types of objects e.g. Scene, Mesh, LineSegments etc.. All these classes are derived from Object3D. It would not make sense to put all the properties of these classes to Object3D.d.ts, right? Instead, the following line needs to be changed:

three.js/src/loaders/ObjectLoader.d.ts

Line 16 in 09e1fde

onLoad?: (object: Object3D) => void,

[jeffreylanters]

I see what is going on. Thanks for clearing it up! A solution for this could be a generic key of type, function to keep it more strict.

[Mugen87]

Similar to this code?

three.js/src/loaders/ObjectLoader.d.ts

Line 22 in 09e1fde

parse<T extends Object3D>(json: any, onLoad?: (object: Object3D) => void): T;

[mrdoob]

It would not make sense to put all the properties of these classes to Object3D.d.ts, right?

Wouldn't this work?

onLoad?: (object: Scene | Mesh | Line | LineSegments | Points) => void,

[Mugen87]

As an alternative, I think it would also work to define a generic type derived from Object3D.

onLoad?: <T extends Object3D>(object: T) => void,

Not sure which approach is better 🤔

[mrdoob]

Not sure which approach is better 🤔

@jeffreylanters you choose 🙂

[jeffreylanters]

As an alternative, I think it would also work to define a generic type derived from Object3D.

onLoad?: <T extends Object3D>(object: T) => void,

Not sure which approach is better 🤔

Like I said before, I think a generic approach would be the best way to go. I think this looks is perfect. Thanks for thinking with me!

[jeffreylanters]

I've updated the PR.

[mrdoob]

Thanks!