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: Watching Native HTML Attributes requires at least one @Prop() declaration #5854

Closed
3 tasks done
rvantonisse opened this issue Jun 25, 2024 · 4 comments · Fixed by #5855 or ionic-team/ionic-framework#29666
Closed
3 tasks done
Assignees
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@rvantonisse
Copy link

rvantonisse commented Jun 25, 2024

Prerequisites

Stencil Version

4.18.3

Current Behavior

I want to @Watch() the host id attribute, but it requires at least one @Prop() set.
Without a @Prop() set, the @Watch() will not trigger.
With any @Prop() set, @Watch("id") will trigger on attribute change.

Expected Behavior

Preferred:
Setting an attribute watcher like @Watch("id"), triggers when changing the attribute;
Without requiring any @Prop() set.

Second best:
Might also agree with requiring at least an equal property like @Prop("id") as it is an attribute expected to be set on the component. But this might require additional code to reflect the exact HTML attribute behavior. (This side-effect is warned for by stencil's console output)
But in this case @Watch("id") is expected to only trigger when @Prop("id") is exact set.

System Info

System: node 20.11.1
    Platform: darwin (23.5.0)
   CPU Model: Apple M3 Pro (12 cpus)
    Compiler: _PROJECT_/node_modules/@stencil/core/compiler/stencil.js
       Build: 1716921701
     Stencil: 4.18.3 😄
  TypeScript: 5.4.5
      Rollup: 2.56.3
      Parse5: 7.1.2
      jQuery: 4.0.0-pre
      Terser: 5.31.0
     Browser: Google Chrome 126.0.6478.114


### Steps to Reproduce

- Create starter Stencil project
- Generate simple component
- Add attribute `@Watch("id")` with corresponding handler
- Add the component to the `index.html`
- Start stencil `npm start`
- Open page and inspect web component
- Change / add the id attribute on the web component
- id watcher did not trigger
- Add a random `@Prop() foo` to component file
- Change the web component `id` attribute again
- `id` watcher did trigger

Component code:
```jsx
import { Component, Host, Element, Watch, h, Prop } from '@stencil/core';

@Component({
  tag: 'test-input',
  styleUrl: 'test-input.css',
  shadow: true,
})
export class TestInput {
  @Element() hostElement: HTMLElement;

  private inputElement: HTMLInputElement;
  
  // Comment @Prop() to see @Watch("id") not working
  @Prop() fakeProp: string;

  @Watch("id")
  handleIdWatch(newId: HTMLElement["id"], currentId: HTMLElement["id"]) {
    console.log(`idWatcher: ${currentId} ---> ${newId}`);
    if (newId !== currentId) {
      // set new input id
      this.inputElement.id = newId;
    }
  }

  render() {
    return (
      <Host>
        <slot></slot>
        <input type="text" id={this.hostElement.id} ref={element => this.inputElement = element} />
      </Host>
    );
  }
}

Code Reproduction URL

https://github.com/rvantonisse/reproduction-stencil-issues/tree/rvantonisse/bug/5854-watching-native-html-attributes-requires-prop

Additional Information

I cant share my work code repo so I need to recreate a this code with a private account.

For now an empty repo, will fill later today.

EDITS

  • [2024-06-25]: Added code reproduction URL
@ionitron-bot ionitron-bot bot added the triage label Jun 25, 2024
@rvantonisse rvantonisse changed the title bug: Watching Native HTML Attributes requires at least one component property set bug: Watching Native HTML Attributes requires at least one @Prop() declaration Jun 25, 2024
@rvantonisse
Copy link
Author

Additionally tested in Browser Firefox 127.0.1

In src/components/test-watch-with-prop/test-watch-with-prop.tsx, resulted with @Watch("id") triggering twice;
Changing from current id to null and then from null to new id.

1: TestWatchWithProp: watch-with-prop ---> null
2: TestWatchWithProp: null ---> other-id

@tanner-reits tanner-reits self-assigned this Jun 25, 2024
@tanner-reits
Copy link
Member

@rvantonisse I was able to confirm this issue. Gonna get this ingested into our backlog so we can take a closer look! Thanks!

@tanner-reits tanner-reits added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed triage labels Jun 25, 2024
tanner-reits added a commit that referenced this issue Jun 25, 2024
github-merge-queue bot pushed a commit that referenced this issue Jun 25, 2024
* fix(runtime): allow watchers to fire w/ no Stencil members

Fixes #5854

STENCIL-1338

* resolve test failures
@tanner-reits
Copy link
Member

A fix for this was included in today's v4.19.0 release!

github-merge-queue bot pushed a commit to ionic-team/ionic-framework that referenced this issue Jun 26, 2024
### Release Notes

<details>
<summary>ionic-team/stencil (@&#8203;stencil/core)</summary>

###
[`v4.19.0`](https://togithub.com/ionic-team/stencil/blob/HEAD/CHANGELOG.md#-4190-2024-06-26)

[Compare
Source](https://togithub.com/ionic-team/stencil/compare/v4.18.3...v4.19.0)

### Bug Fixes

* **compiler:** support rollup's external input option
([#3227](ionic-team/stencil#3227))
([2c68849](ionic-team/stencil@2c68849)),
fixes [#3226](ionic-team/stencil#3226)
* **emit:** don't emit test files
([#5789](ionic-team/stencil#5789))
([50892f1](ionic-team/stencil@50892f1)),
fixes [#5788](ionic-team/stencil#5788)
* **hyrdate:** support vdom annotation in nested dsd structures
([#5856](ionic-team/stencil#5856))
([61bb5e3](ionic-team/stencil@61bb5e3))
* label attribute not toggling input
([#3474](ionic-team/stencil#3474))
([13db920](ionic-team/stencil@13db920)),
fixes [#3473](ionic-team/stencil#3473)
* **mock-doc:** expose ShadowRoot and DocumentFragment globals
([#5827](ionic-team/stencil#5827))
([98bbd7c](ionic-team/stencil@98bbd7c)),
fixes [#3260](ionic-team/stencil#3260)
* **runtime:** allow watchers to fire w/ no Stencil members
([#5855](ionic-team/stencil#5855))
([850ad4f](ionic-team/stencil@850ad4f)),
fixes [#5854](ionic-team/stencil#5854)
* **runtime:** catch errors in async lifecycle methods
([#5826](ionic-team/stencil#5826))
([87e5b33](ionic-team/stencil@87e5b33)),
fixes [#5824](ionic-team/stencil#5824)
* **runtime:** don't register listener before connected to DOM
([#5844](ionic-team/stencil#5844))
([9d7021f](ionic-team/stencil@9d7021f)),
fixes [#4067](ionic-team/stencil#4067)
* **runtime:** properly assign style declarations
([#5838](ionic-team/stencil#5838))
([5c10ebf](ionic-team/stencil@5c10ebf))
* **testing:** allow to re-use pages across it blocks
([#5830](ionic-team/stencil#5830))
([561eab4](ionic-team/stencil@561eab4)),
fixes [#3720](ionic-team/stencil#3720)
* **typescript:** remove unsupported label property
([#5840](ionic-team/stencil#5840))
([d26ea2b](ionic-team/stencil@d26ea2b)),
fixes [#3473](ionic-team/stencil#3473)


### Features

* **cli:** support generation of sass and less files
([#5857](ionic-team/stencil#5857))
([1883812](ionic-team/stencil@1883812)),
closes [#2155](ionic-team/stencil#2155)
* **compiler:** generate export maps on build
([#5809](ionic-team/stencil#5809))
([b6d2404](ionic-team/stencil@b6d2404))
* **complier:** support type import aliasing
([#5836](ionic-team/stencil#5836))
([7ffb25d](ionic-team/stencil@7ffb25d)),
closes [#2335](ionic-team/stencil#2335)
* **runtime:** support declarative shadow DOM
([#5792](ionic-team/stencil#5792))
([c837063](ionic-team/stencil@c837063)),
closes [#4010](ionic-team/stencil#4010)
* **testing:** add `toHaveLastReceivedEventDetail` event spy matcher
([#5829](ionic-team/stencil#5829))
([63491de](ionic-team/stencil@63491de)),
closes [#2488](ionic-team/stencil#2488)
* **testing:** allow to disable network error logging via
'logFailingNetworkRequests' option
([#5839](ionic-team/stencil#5839))
([dac3e33](ionic-team/stencil@dac3e33)),
closes [#2572](ionic-team/stencil#2572)
* **testing:** expose captureBeyondViewport in pageCompareScreenshot
([#5828](ionic-team/stencil#5828))
([cf6a450](ionic-team/stencil@cf6a450)),
closes [#3188](ionic-team/stencil#3188)

</details>
@yigityuce
Copy link
Contributor

yigityuce commented Sep 6, 2024

hey I know it's already implemented, however, we were having the same issue in our project and we were able to fix it by using attribute config in the @Prop annotation such as:

  /**
   * Role of the host element which is used for accessibility
   * @ignore
   */
  @Prop({ attribute: 'role' }) hostRole: 'textbox' | 'combobox' = 'textbox';


  /**
   * ID of the host element which is used for accessibility
   * @ignore
   */
  @Prop({ attribute: 'id' }) hostId: string = generateElementId(this.hostElement);

I just wanted to state it for the ones who are not able to upgrade to v4.19.0

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
3 participants