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.

Sign up for GitHub

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

Jump to bottom

[scoped-custom-element-registry] toggleAttribute should only trigger attributeChangedCallback when the attribute is actually toggled #556

Open
3 of 5 tasks
jessevanassen opened this issue Aug 22, 2023 · 0 comments · May be fixed by #557
Open
3 of 5 tasks

[scoped-custom-element-registry] toggleAttribute should only trigger attributeChangedCallback when the attribute is actually toggled #556

jessevanassen opened this issue Aug 22, 2023 · 0 comments · May be fixed by #557
Labels
Focus Area: Standards & Polyfills Type: Bug

Comments

@jessevanassen
Copy link

Description

According to the toggleAttribute(qualifiedName, force) spec, toggling an attribute should only result in a change if the attribute actually changes. In other words, if the attribute is already present and force is true or if the attribute is not present and force is false, the attribute shouldn't change.

When the polyfill is loaded, the attribute is always changed, resulting in the attributeChangedCallback firing too frequently.

This is also inconsistent with how it works out of the box in the latest versions of Chrome, Firefox and Edge.

It is especially problematic, because it breaks the spec for all web components on the page, not just components that use the scoped registry.
In our case, it lead to an unrelated piece of code behaving differently when the polyfill was loaded elsewhere.

Example

<!DOCTYPE html>
<html lang="en">
	<head>
		<meta charset="UTF-8">
		<script src="https://unpkg.com/@webcomponents/[email protected]/src/scoped-custom-element-registry.js"></script>
	</head>
	<body>
		<script type="text/javascript">
			customElements.define('my-element', class MyElement extends HTMLElement {
				static get observedAttributes() {
					return ['param'];
				}

				attributeChangedCallback(name, oldValue, newValue) {
					console.log('attributeChangedCallback', { name, oldValue, newValue });
				}
			});


			const element = document.createElement('my-element');
			document.body.append(element);

			// This should log: first setting of param
			element.setAttribute('param', '');

			// This should log: removing the param
			element.toggleAttribute('param', false);

			// This shouldn't log: parameter is not present
			element.toggleAttribute('param', false);

			// This should log: adding the parameter 
			element.toggleAttribute('param', true);

			// This shouldn't log: parameter is already present
			element.toggleAttribute('param', true);
		</script>
	</body>
</html>
  • When the polyfill is not included, the attributeChangedCallback fires 3 times, resulting in 3 log statements.
  • When the polyfill is included, the attributeChangedCallback fires 5 times, resulting in 5 log statements.

Expected behavior

attributeChangedCallback is only called when the attribute is actually toggled

Actual behavior

attributeChangedCallback is also called when the attribute presence remains the same

Version

0.0.9

Browsers affected

  • Chrome
  • Firefox
  • Edge
  • Safari
  • IE 11

(These are the only browsers I can test)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Focus Area: Standards & Polyfills Type: Bug
Projects
None yet
Milestone
No milestone
1 participant
@jessevanassen