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: Prop() is mutable despite what it says in the docs #6006

Open
3 tasks done
rramsey opened this issue Oct 2, 2024 · 4 comments
Open
3 tasks done

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

rramsey opened this issue Oct 2, 2024 · 4 comments
Labels
Resolution: Refine This PR is marked for Jira refinement. We're not working on it - we're talking it through.

Comments

@rramsey
Copy link

rramsey commented Oct 2, 2024

Prerequisites

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

  1. Create a new StencilJS component with npm init stencil@latest called prop-bug.
  2. cd prop-bug and edit the package.json to use "@stencil/core": "^4.21.0", and run npm install
  3. 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.
  4. 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
  5. Add another button that directly changes this.last.
  6. At the command line, run npm install and npm start
  7. Open new private tabs Chrome 128.0, Firefox 102.5 esr, or Edge 107.0
  8. 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.
  9. Enter a new first name and click the button or click the button to change the last name and watch the console.
  10. 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
  1. 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>
   ;
}
@ionitron-bot ionitron-bot bot added the triage label Oct 2, 2024
@tanner-reits
Copy link
Member

@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

@tanner-reits tanner-reits added Resolution: Refine This PR is marked for Jira refinement. We're not working on it - we're talking it through. and removed triage labels Oct 4, 2024
@rramsey
Copy link
Author

rramsey commented Oct 7, 2024

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Refine This PR is marked for Jira refinement. We're not working on it - we're talking it through.
Projects
None yet
Development

No branches or pull requests

4 participants
@christian-bromann @rramsey @tanner-reits and others