-
Notifications
You must be signed in to change notification settings - Fork 793
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
fix: runtime decorators #6076
base: main
Are you sure you want to change the base?
fix: runtime decorators #6076
Conversation
# Conflicts: # src/runtime/dom-extras.ts
…ns/stencil into fix-runtime-decorators
💯 agree here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome change set 🙌 thanks a lot!
Could we add some docs for custom decorator support? I would suggest to add a Custom Decorator
section here and show case a simple real world example. What do you think?
@christian-bromann when testing different run-time decorator patterns someone might use with Stencil decorated members, I found more examples of Stencil 'getting-in-the-way' ... not broken - just not particularly flexible or useful. So before writing docs with examples, I'm gonna submit one more MR which will make user-land decorators 100% more useful in Stencil. Hope that's cool! |
💯 cool! |
With the introduction of #3614, Stencil started removing
@State
/@Prop
class fields during transpilation only to re-add them later, during bundling, as constructor initialised values.This was due to a new behaviour with JS classes, given tsconfig:
A typical Stencil component:
Compiled output went from:
to:
A side-effect this behaviour is user-defined, run-time decorators targeting
@State
/@Prop
would be stripped out during transpilation and break expected behaviour.Within the attached issue, the Stencil dev (at-the-time) outlined the reason for the broken behaviour and also stated their reticence to fix it; a preference to move away from Stencil's statically-analysed decorators for real-life, run-time decorators. Getting class fields working again within Stencil would just be layering-on more tech-debt when the ultimate goal should be to remove it.
Whilst I am completely on-board with the dev's reasoning - the more magic that Stencil can remove, the more Stencil can 'get-out-of-the-way' of native behaviour and expectations - the better. However, I also believe:
experimentalDecorators
and / or the not-yet supported ECMA compliant decorators - will not be small and will take time.What is the current behavior?
GitHub Issue Number: #3831
What is the new behavior?
Fixes #3831
@State
/@Prop
decorated fields with the original Stencilget
/set
during run-time only if it finds modern class static initialisers (during build time we detect them and switch on a newmodernPropertyDecls
build flag)Documentation
Does this introduce a breaking change?
Testing
modernPropertyDecls
target: "es2022"
combined withdist
,dist-custom-elements
and run-time decoratorsOther information