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

Svelte 5: Incorrect Compiler Output With Private Fields #13965

Closed
abdel-17 opened this issue Oct 27, 2024 · 3 comments · Fixed by #14378
Closed

Svelte 5: Incorrect Compiler Output With Private Fields #13965

abdel-17 opened this issue Oct 27, 2024 · 3 comments · Fixed by #14378
Assignees
Labels

Comments

@abdel-17
Copy link

Describe the bug

class Box {
	#value = $state();

	swap(other) {
		const value = this.#value;
		this.#value = other.value;
		other.#value = value;
	}
}

The compiler converts other.#value = value; to $.get(other.#value) = value;.

Reproduction

https://svelte.dev/playground/untitled?version=5.1.3#H4sIAAAAAAAAE22OQWrDMBBFrzJMu3DAOHsnKTRddZUDVF0oYkoMiiQ0366L8d2L6jqk0NUw778_zMTBXoVbfg0u5iwO9BKvqfOS6dQj9eCaPzovyu3bxPhKRS6A67X6nFKjg3gUdrYq_3EXAyRAueW9utwlPJlg4LxVpWMcaSqrwcNgfS90oEeFhVSbnQlLop82VREXyZtVNnAxKGjt4NJps1zYrcYdowP99Ju_xsJuyn04lzGbsN_-_sw1Q0Zwi9zL_D5_A7TBU9ZAAQAA

Logs

No response

System Info

Not necessary

Severity

annoyance

@dummdidumm dummdidumm added the bug label Oct 27, 2024
@abdel-17
Copy link
Author

abdel-17 commented Oct 31, 2024

Not sure if my input is helpful, but I see two potential ways to fix this issue.

Solution 1:
Don't mess with how getters and setters are called.

Although, this might slightly decrease performance, it avoids weird bugs like this from coming up in the future. You would have to store the itself state in a separate private variable.

class Box {
	#var0 = $.state();

	get #value() {
		return $.get(this.#var0);
	}

	set #value(value) {
		$.set(this.#var0, value);
	}

	swap(other) {
		const value = this.#value;
		this.#value = other.value;
		other.#value = value;
	}
}

Solution 2:
Handle private field access for external variables differently.

An admittedly hacky way to solve this is to first check if the field a signal first. You can also skip this step for private fields that aren't declared as $state.

class Box {
	#value = $.state();

	swap(other) {
		const value = this.#value;
		this.#value = other.value;
		if ($.is_state(other.#value)) {
			$.set(other.#value, value);
		} else {
			other.#value = value;
		}
	}
}

@paoloricciuti paoloricciuti self-assigned this Oct 31, 2024
@abdel-17
Copy link
Author

I really don't want to be annoying, but is there any progress on this issue? I worked around it for now by manually defining setters Java-style.

class Box {
  #setValue(value) {
    this.#value = value;
  }
}

@paoloricciuti
Copy link
Member

I really don't want to be annoying, but is there any progress on this issue? I worked around it for now by manually defining setters Java-style.

class Box {
  #setValue(value) {
    this.#value = value;
  }
}

Don't worry, I actually forgot about this issue so good that you pinged hete... I'll look into it tomorrow if I can

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants