Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 67 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"devDependencies": {
"@lit-labs/testing": "^0.2.1",
"@open-wc/testing": "^3.1.7",
"@playwright/test": "^1.45.0-alpha-2024-05-24",
"@open-wc/testing-helpers": "^1.7.1",
"@rollup/plugin-node-resolve": "^7.1.3",
"@rollup/plugin-typescript": "^6.0.0",
Expand Down
3 changes: 0 additions & 3 deletions src/element-internals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -393,9 +393,6 @@ export function forceElementInternalsPolyfill(forceCustomStateSet = true) {
} else if (this.tagName.indexOf('-') === -1) {
throw new Error(`Failed to execute 'attachInternals' on 'HTMLElement': Unable to attach ElementInternals to non-custom elements.`);
}
if (internalsMap.has(this)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the right fix. This is removing a guard against attaching a second internals instance. I think a better fix would be to not call upgrade internals if an instance it doesn't exist.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this, and removing the line didn't work, as the fix for upgradeInternals is needed to run for tests to pass with Stencil components with Jest, so in my take on it in #142 I added a flag to make to not throw the error if attachInternals is run in the upgradeInternals function

throw new DOMException(`DOMException: Failed to execute 'attachInternals' on 'HTMLElement': ElementInternals for the specified element was already attached.`);
}
return new ElementInternals(this) as IElementInternals;
}
}
Expand Down
13 changes: 10 additions & 3 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export const formInputCallback = (event: Event) => {
* @param {Event} - The form change event
* @return {void}
*/
export const formChangeCallback = (event: Event) => {
export const formChangeCallback = (event: Event) => {
setFormValidity(findParentForm(event.target));
};

Expand Down Expand Up @@ -263,7 +263,7 @@ export const throwIfNotFormAssociated = (ref: ICustomElement, message: string, E
* @param method {'checkValidity'|'reportValidity'} - The original method
* @returns {boolean} The form's validity state
*/
export const overrideFormMethod = (form: HTMLFormElement, returnValue: boolean, method: 'checkValidity'|'reportValidity'): boolean => {
export const overrideFormMethod = (form: HTMLFormElement, returnValue: boolean, method: 'checkValidity' | 'reportValidity'): boolean => {
const elements = formElementsMap.get(form);

/** Some forms won't contain form associated custom elements */
Expand All @@ -287,10 +287,17 @@ export const overrideFormMethod = (form: HTMLFormElement, returnValue: boolean,
*/
export const upgradeInternals = (ref: ICustomElement) => {
if (ref.constructor['formAssociated']) {
const internals = internalsMap.get(ref);
let internals = internalsMap.get(ref);
// we might have cases where the internals are not set
if (internals === undefined) {
console.warn('ElementInternals missing from the element', ref);
ref.attachInternals();
internals = internalsMap.get(ref);
}
const { labels, form } = internals;
initLabels(ref, labels);
initForm(ref, form, internals);

}
};

Expand Down
10 changes: 8 additions & 2 deletions test/FormElements.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { expect, fixture, html } from '@open-wc/testing';
import '../dist/index.js';

class TestInput extends HTMLElement {
static formAssociated = true;
internals = this.attachInternals();
Expand All @@ -11,7 +10,12 @@ customElements.define('test-input', TestInput);
class TestDummy extends HTMLElement {
}


customElements.define('test-dummy', TestDummy);
class TestFormAssociatedNoAttachInternals extends HTMLElement {
static formAssociated = true;
}
customElements.define('test-no-attach-internals', TestFormAssociatedNoAttachInternals);

async function createForm(): Promise<HTMLFormElement> {
return await fixture<HTMLFormElement>(html`
Expand All @@ -22,18 +26,20 @@ async function createForm(): Promise<HTMLFormElement> {
<test-input name="second" id="ti2"></test-input>
<test-input name="third" id="ti3"></test-input>
<button type="submit">Submit</button>
<test-no-attach-internals></test-no-attach-internals>
</form>`);
}

it('must contains the custom elements associated to the current form, in the correct order', async () => {
const form = await createForm();
expect(form.elements).to.have.length(5);
expect(form.elements).to.have.length(6);

expect(form.elements[0]).to.be.an.instanceof(HTMLInputElement);
expect(form.elements[1]).to.be.an.instanceof(TestInput);
expect(form.elements[2]).to.be.an.instanceof(TestInput);
expect(form.elements[3]).to.be.an.instanceof(TestInput);
expect(form.elements[4]).to.be.an.instanceof(HTMLButtonElement);
expect(form.elements[5]).to.be.an.instanceof(TestFormAssociatedNoAttachInternals);

expect(form.elements[0].id).to.equal('foo');
expect(form.elements[1].id).to.equal('ti1');
Expand Down