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

polyfill not robust for elements that do not call attachInternals (e.g. -buttons) #127

Open
christophe-g opened this issue Mar 1, 2024 · 2 comments · May be fixed by #130
Open

polyfill not robust for elements that do not call attachInternals (e.g. -buttons) #127

christophe-g opened this issue Mar 1, 2024 · 2 comments · May be fixed by #130

Comments

@christophe-g
Copy link

christophe-g commented Mar 1, 2024

Hey - thanks a lot for this polyfill !

I run into a situation where elements that have a formAssociated = true , but do not call attachInternals in their constructor.

For example: https://github.com/material-components/material-web/blob/c3d303eaac1e38e48ea138e0eec034bffdc84c4b/button/internal/button.ts#L42

The polyfill breaks when those elements are attached to the DOM and call upgradeInternals :

	const upgradeInternals = (ref) => {
			if (ref.constructor['formAssociated']) {
					const internals = internalsMap.get(ref);
					const { labels, form } = internals;
					initLabels(ref, labels);
					initForm(ref, form, internals);
			}
	};

as internals is undefined in this case, the app throws with:
Uncaught TypeError: Cannot destructure property 'labels' of 'internals' as it is undefined.

Those elements are then not rendered.

@christophe-g
Copy link
Author

christophe-g commented May 2, 2024

@calebdwilliams - I can submit a PR for this, just tell me if it has any chances of being be reviewed.

It basically rewrites upgradeInternals as

export const upgradeInternals = (ref: ICustomElement) => {
  if (ref.constructor['formAssociated']) {
    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);

  }
};

@calebdwilliams
Copy link
Owner

Yeah, I’ll look at it. Submit a PR and make sure there’s a regression test.

christophe-g added a commit to preignition/element-internals-polyfill that referenced this issue May 24, 2024
@christophe-g christophe-g linked a pull request May 24, 2024 that will close this issue
christophe-g added a commit to preignition/element-internals-polyfill that referenced this issue May 24, 2024
christophe-g added a commit to preignition/element-internals-polyfill that referenced this issue May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants