Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inheritance support for Datex classes #131

Open
jonasstrehle opened this issue Sep 15, 2024 · 2 comments
Open

Inheritance support for Datex classes #131

jonasstrehle opened this issue Sep 15, 2024 · 2 comments
Labels
bug Something isn't working essential

Comments

@jonasstrehle
Copy link
Member

jonasstrehle commented Sep 15, 2024

Structs / classes do not work with inheritance combined with Storage collections.

import { Datex, property } from "unyt_core/datex.ts";
import { inferType, StorageSet } from 'unyt_core/datex_all.ts';

const C = 0;

{ // Classes
	@sync class A {
		@property a!: number;
		construct() {
			this.a = 42;
		}
	}
	@sync class B extends A {
		@property b!: number;
		construct() {
			super.construct();
			this.b = 69;
		}
	}
	const list = eternalVar("tmp1-"+C) ?? $$(new StorageSet<A>());
	if (await list.getSize() === 0) {
		await list.add(new A());
		await list.add(new B());
	}
	for await (const entry of list) {
		console.log(entry.a, entry.b)
	}
	console.log("")
}

{ // Structs
	const A = struct(
		class A {
			@property a!: number;
			construct() {
				this.a = 42;
			}
		}
	)
	type A = inferType<typeof A>;
	
	const B = struct(
		class B extends A {
			@property b!: number;
			construct() {
				super.construct();
				this.b = 69;
			}
		}
	)
	type B = inferType<typeof B>;
	const list = eternalVar("tmp2-"+C) ?? $$(new StorageSet<A>());
	if (await list.getSize() === 0) {
		await list.add(new A());
		await list.add(new B());
	}
	for await (const entry of list) {
		console.log(entry.a, entry.b)
	}
}
@benStre
Copy link
Member

benStre commented Sep 29, 2024

As far as I can tell after some investigation, this is not a new issue but the consequence of the new decorators which don't allow declare for class properties.

When defining @property b!: number;, b will be overridden and set to undefined when a new instance (even when reconstructing) is created.

We fixed this by calling a property initializer (INIT_PROPS) after the instantiation.
This is called after the instantiation of the parent class A, since the properties are already required (e.g. in the constructor of A) at this point. But afterwards, when B is initialized, the previously initialized properties are set to undefined again.

Previously, we only allowed INIT_PROPS to be called once per instantiation. I removed this restriction now, meaning that for inherited classes, the property initialization is executed multiple times. I can't assure that this has no side effects, but I assume we only added this restriction for performance reasons.

@benStre
Copy link
Member

benStre commented Sep 29, 2024

-> not a storage collection or DATEX issue, but a DATEX-JS adapter issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working essential
Projects
None yet
Development

No branches or pull requests

2 participants