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

bug: private state can conflict with global HTML attributes #3134

Open
3 tasks done
willmartian opened this issue Nov 8, 2021 · 6 comments
Open
3 tasks done

bug: private state can conflict with global HTML attributes #3134

willmartian opened this issue Nov 8, 2021 · 6 comments
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@willmartian
Copy link
Contributor

willmartian commented Nov 8, 2021

Prerequisites

Stencil Version

2.10.0

Current Behavior

In the custom elements build, declaring variables on a component with the same name as a global HTML attribute will cause an error upon creating the component programmatically ala someting like document.createElement: Uncaught DOMException: Failed to construct 'CustomElement': The result must not have attributes

For example, given the following Stencil component:

  class MyComponent {
     private inputMode = false;
  }

Building with the custom elements output target will generate the following:

  class MyComponent extends HTMLElement {
    constructor() {
      super();
      this.inputMode = false;
    }
  }

This violates the HTML standard's requirements for a custom element constructor:
(from https://html.spec.whatwg.org/multipage/custom-elements.html#custom-element-conformance)

The element's attributes and children must not be inspected, as in the non-upgrade case none will be present, and relying on upgrades makes the element less usable.

The element must not gain any attributes or children, as this violates the expectations of consumers who use the createElement or createElementNS methods.

Expected Behavior

I would expect Stencil to either namespace private variables to prevent this type of unintended collision, or cause a build error when declaring private variables with colliding names. The @Prop() decorator will already cause a warning if it detects a global attribute name is being used, but private vars and @State() do not.

Steps to Reproduce

I am seeing this issue in Ionic Frameworks Vue e2e tests, from this private state declaration: /~https://github.com/ionic-team/ionic-framework/blob/next/core/src/components/picker-internal/picker-internal.tsx#L21

This is because inputMode is a global attribute: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/inputmode

Code repro steps:

  1. git clone /~https://github.com/willmartian/stencil-ce-repro.git
  2. In stencil_lib run npm ci && npm run build && npm pack
  3. In ce_test_site run npm ci && npm run start

Code Reproduction URL

/~https://github.com/willmartian/stencil-ce-repro

Additional Information

No response

@markcellus
Copy link

Good catch. Shouldn't the constructor have a super() call? 🤔

@willmartian
Copy link
Contributor Author

Good catch. Shouldn't the constructor have a super() call? 🤔

Yep! Updated the code block.

@rwaskiewicz
Copy link
Contributor

👋 I took some time to look through the reproduction case, and wasn't able to reproduce this issue. But, I think there may be some wiring-up of things not working here - it doesn't appear that the components in the reproduction case are being auto-defined properly when I run the repro steps:
Screen Shot 2021-11-09 at 8 38 47 AM
(I'd expect the error message you mentioned in the console).

I had to tweak your install instructions a bit, as there were checksum mismatches installing the tarball trying to run npm ci in step 3:

1.  git clone /~https://github.com/willmartian/stencil-ce-repro.git
2.  In stencil_lib run npm ci && npm run build && npm pack
- 3. In ce_test_site run npm ci && npm run start
+ 3. In ce_test_site run npm i ../stencil_lib/stencil-test-project-0.0.1.tgz && npm ci && npm start

Is there anything I'm missing there? Any place I should be looking elsewhere for this error message?

@rwaskiewicz rwaskiewicz added Awaiting Reply This PR or Issue needs a reply from the original reporter. Repro: Yes labels Nov 9, 2021
@willmartian
Copy link
Contributor Author

willmartian commented Nov 9, 2021

Huh, doing the above repro and loading the same network resources, I get the following:

image

Seeing this in Chrome, FF, and Safari.

@ionitron-bot ionitron-bot bot added Reply Received and removed Awaiting Reply This PR or Issue needs a reply from the original reporter. labels Nov 9, 2021
@rwaskiewicz
Copy link
Contributor

Discussed this offline with @willmartian - turns out this isn't reproducible in Firefox v94, but is on FF 95 (and other major browsers)

@rwaskiewicz rwaskiewicz added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed Bug: Needs Validation labels Nov 15, 2021
@subgan82
Copy link

Any update on this issue , we also see this same issue with some of our components.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Development

No branches or pull requests

5 participants