ionic-team / stencil

A toolchain for building scalable, enterprise-ready component systems on top of TypeScript and Web Component standards. Stencil components can be distributed natively to React, Angular, Vue, and traditional web developers from a single, framework-agnostic codebase.
https://stenciljs.com
Other
12.56k stars 784 forks source link

@Prop decorator does not work on a getter/setter #1359

Open apillard opened 5 years ago

apillard commented 5 years ago

Stencil version:

 @stencil/core@0.17.2

I'm submitting a:

[ ] bug report [X] feature request [ ] support request => Please do not submit support requests here, use one of these channels: https://stencil-worldwide.herokuapp.com/ or https://forum.ionicframework.com/

Current behavior:

When adding a \@Prop decorator to a getter/setter property, the code compiles, and the property is added to the generated interface, but accessing the prop in the client js does not execute the getter, and setting the value does not execute the setter.

To give you an example of where this would be useful, I'm working on a set of mapping components and have a property for the zoom level. The zoom property allows the user to set a zoom programatically and also exposes the current zoom that may be a result of end user mouse interaction with the map. Using the scroll wheel will change the zoom level too.

Currently every time the map is zoomed by mouse interaction, I have to sync the stencil zoom prop with the zoom from my internal mapping api. This causes complications because the @Watch decorator also fires when the value is modified by the component internally. See StackOverflow link below. If getter/setter was supported, the stencil prop could act as a more direct passthrough, and I could fetch the zoom level from the internal api only when it's requested.

Expected behavior:

Steps to reproduce:

Related code:

@Prop()
get zoom(): number {
return 10;
}
set zoom(newZoom: number) {
console.log('setting the zoom')
}

Other information:

Follow up to discussion here: https://github.com/ionic-team/stencil/issues/230 Opening this as a new ticket because it should be a \@Prop decorator and not a \@Watch

Also see this StackOverflow post that describes problems you run into when you have to make the prop mutable and sync it by modifying the value. https://stackoverflow.com/questions/52229575/how-to-mutate-a-property-within-a-stenciljs-component-without-triggering-watch

gtranter commented 5 years ago

Any traction on this yet? It is still an issue as of v1.1.7. This would be extremely helpful for use in deprecating API. For example:

// supersedes oldProp 
@Prop({mutable: true}) newProp: string;

// deprecated in favor of newProp
@Prop() 
get oldProp(): string {
    return this.newProp;
}
set oldProp(propValue: string) {
    this.newProp = propValue;
}

Property renaming is obviously the simplest of use cases for this - it could be leveraged to do more complicated things.

mattkenefick commented 5 years ago

I too would like to see this happen.. even if there something like..

@Prop({
    set: (value) => {
        ...
    }
})
gforceg commented 4 years ago

I checked out #230 as well and I am having trouble finding any documentation of @PropWillChange or @PropDidChange.

But I did find a reference to their usage in #287.

Unfortunatley these decorators are no longer exported from @stencil/core.

image

Do those decorators still exist?

gforceg commented 4 years ago

Ah, nevermind. It looks like These were merged into @Watch

markcellus commented 4 years ago

Not sure why this issue was closed. Did anyone ever get this working?

dogoku commented 4 years ago

Hi @manucorporat

I would also like to know why this issue was closed? Here are some valid use cases this feature will provide:

If it's an implementation issue due to overloading @Prop decorator, maybe introducing new decorators like @Getter/@Setter('propName') would make it easier to implement

markcellus commented 4 years ago

https://github.com/ionic-team/stencil/issues/230 also seems related.

@adamdbradley can you provide any insight here on why this ticket was closed? Me and my team would like to take advantage of gets and sets in the spec. Right now, using Stencil prevents us from doing so.

xe4me commented 4 years ago

This can also be very helpful when using JSON.parse to parse the passed string, when the string is not a primitive.

TheCelavi commented 3 years ago

Hi @manucorporat - I believe that this should be revised. Having get/set for Prop is a valid request, we have stumbled upon this sooooo many times, it is ridiculous what we have to do and what we have to hack in order for this to make it work.

99,99% use cases are in regards to component composition. Examples are

Example for my-component:

In such scenario, using Prop in combination with Watch end event listeners is just such overkill!

document.querySelector('my-component').value = 'some value' -> in such case (component composition, proxy), re-rendering on 'my-component' is overkill, no need for that, value should be just passed to proxied element (i.e. INPUT).

Same about getting value from INPUT.

The proper component composition is virtually impossible without enabling getter/setter for prop.

Would you please provide us with reasoning why this feature is dismissed? There must be a strong reason for it, because, it is very clear that supporting getter/setter for props is just a natural thing.

claviska commented 3 years ago

Reopening this one so we can reevaluate, as it seems to be heavily requested. @manucorporat do you remember if this was closed due to a technical limitation?

TheCelavi commented 3 years ago

@claviska Ok, here is one more example, proxying VIDEO/AUDIO element and its "currentTime" property. "currentTime" property constantly changes as video/audio plays. In component composition, it is ridiculous to have a prop with the name "currentTime" and pass value/listen to value change and update VIDEO/AUDIO element/parent element.

There is no reason to trigger change detection there.

With get/set or some other "property pass through" solution, we would be in a position to use some basic components/HTML elements and enrich them through component composition keeping the compatibility with the proxied elements.

For example, I could use VIDEO html tag and you could provide me with an even better video player that enhances it, adding better UI, some more methods/props, but in general, through property proxying there would be 100% compatibility with my existing code.

BTW - thank you for re-opening and reconsidering this feature request.

ionitron-bot[bot] commented 3 years ago

Thanks for the issue! This issue is being closed due to inactivity. If this is still an issue with the latest version of Stencil, please create a new issue and ensure the template is fully filled out.

Thank you for using Stencil!