bug: Prop() is mutable despite what it says in the docs

rramsey commented 1 week ago

Prerequisites

[X] I have read the Contributing Guidelines.
[X] I agree to follow the Code of Conduct.
[X] I have searched for existing issues that already report this problem, without success.

Stencil Version

4.21.0

Current Behavior

The documentation at https://stenciljs.com/docs/properties#mutability says:

A Prop is by default immutable from inside the component logic. Once a value is set by a user, the component cannot update it internally.

Creating a brand new StencilJS component still allows you to mutate a standard Prop() within a component. The component re-renders after the property is changed.

Expected Behavior

I would expect the StencilJS build to fail and report an error that the component attempted to mutate an immutable property
Alternately, I would expect a console.error during development and the property not to change, and when built the property would only change if the value were changed in the Vue/Angular/Html app.
Worst case scenario, I would expect the documentation to say something like:

A Prop is intended to be immutable from inside the component logic. During development, a console warning is produced when a component changes an immutable property. Note that the property is still changed, but the developer is warned. The developer should change the property to allow mutability with @Prop({mutable: true}). Expect mutating a default @Prop() to cause a build failure in future versions of StencilJS

System Info

System: node 20.15.1
    Platform: windows (10.0.22000)
   CPU Model: 12th Gen Intel(R) Core(TM) i7-1265U (12 cpus)
    Compiler: c:\dev\prop-bug\node_modules\@stencil\core\compiler\stencil.js
       Build: 1724698030
     Stencil: 4.21.0
  TypeScript: 5.5.3
      Rollup: 2.56.3
      Parse5: 7.1.2
      jQuery: 4.0.0-pre
      Terser: 5.31.1
Tested in private tabs with Chrome 128.0, Firefox 102.5 esr, and Edge 107.0

Steps to Reproduce

Create a new StencilJS component with npm init stencil@latest called prop-bug.
cd prop-bug and edit the package.json to use "@stencil/core": "^4.21.0", and run npm install
In the ./src/components/my-component/my-component.tsx file add an input field so the user can input a new value for this.first.
Add a button with a function to run when the button is clicked that changes the value of this.first to the value in the input field
Add another button that directly changes this.last.
At the command line, run npm install and npm start
Open new private tabs Chrome 128.0, Firefox 102.5 esr, or Edge 107.0
Open the dev tools so you can see the console and then open http://localhost:3333/ or whatever port your StencilJS app reports it is running on.
Enter a new first name and click the button or click the button to change the last name and watch the console.

The html on the page will change to reflect the new first and last names and the console will report:

@Prop() "last" on <my-component> is immutable but was modified from within the component.
More information: https://stenciljs.com/docs/properties#prop-mutability

Build the component and add it to a new Vue/Angular/Html project and repeat. The properties will still update but the warning won't show in the console.

Code Reproduction URL

https://github.com/rramsey/stencil-bug

Additional Information

The repo uses

render() {
    return (
        <Host>
...
        </Host>
   );
}

But I see the same behavior with a simple:

render() {
    return <div>
...
        </div>
   ;
}

tanner-reits commented 1 week ago

@rramsey Thanks for the feedback! Mutable props is something we've gotten feedback on in the past and is a topic we need to analyze for the future. I'll get this labeled and link some related issues if you wanna check those out and have any feedback on them as well. Thanks again!

Relates to: #5644, #3490

rramsey commented 6 days ago

No worries. I'm just glad I hadn't goofed up and been wrong. Although I wish I had found the other entries when I searched. Sorry for the sort-of dupe.

Honestly, the simplest solution would really be to just change the docs for now to let people know that this is in flux and that people should not rely on mutable: false being truly immutable.

christian-bromann commented 6 days ago

@rramsey would you be open to take a stab at this and provide a PR to our docs project?

ionic-team / stencil