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

Immutable prop warning for various components #124

Open
brettchalupa opened this issue Mar 17, 2021 · 9 comments
Open

Immutable prop warning for various components #124

brettchalupa opened this issue Mar 17, 2021 · 9 comments

Comments

@brettchalupa
Copy link

brettchalupa commented Mar 17, 2021

Stencil version: (run npm list @stencil/core from a terminal/cmd prompt and paste output below):

I'm submitting a ... (check one with "x")
[x] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or https://stencil-worldwide.slack.com

Current behavior:

dozens of console warnings for mutable props being changed are output to the console

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

Expected behavior:

no console warnings

Steps to reproduce:

  1. create a new stencil app or upgrade to stencil 2.4.0, using app from https://stenciljs.com/docs/getting-started yields the results
  2. open the app in the browser
  3. open the web tools

Other information:

Screenshot:

image

Commit that introduced the warning: ionic-team/stencil@9c18fa0

I think the fix would be change the code for these props (example) to be mutable, e.g.

@Prop({ mutable: true }) history?: RouterHistory;
@Prop({ mutable: true }) location?: LocationSegments;
@Prop({ mutable: true }) root?: string;

I'm happy to submit a PR if that's accurate, but I'm not 100% confident in being able to cover the scope of these changes. Thanks!

@fohlin
Copy link

fohlin commented Apr 21, 2021

Good bug report and a shame that it's not getting any attention. Should encourage people willing to do PRs!

@MrGowdy
Copy link

MrGowdy commented Jun 23, 2021

Issue still persists. These errors also clutter the 'npm run test' results. It took me a while to figure out that I wasn't doing anything as a new Stencil developer. A PR would be much appreciated!

@ssurabhi10
Copy link

Facing the same issue! A PR would be much appreciated!

@gregorypratt
Copy link

My assumption is that v2 is on it's way so this gets shelved in favour of that re-write

@MrGowdy
Copy link

MrGowdy commented Jul 20, 2021

@gregorypratt I hope you are right.

In the meantime, does anyone know if it's possible to temporarily solve this issue locally and prevent these warnings? It is really cluttering my console while developing.

@Overthane
Copy link

@gregorypratt I hope you are right.

In the meantime, does anyone know if it's possible to temporarily solve this issue locally and prevent these warnings? It is really cluttering my console while developing.

I just went into node_modules/@stencil/router/collection/components/ and changed mutable: false to mutable: true for the appropriate Props, in the appropriate components.

@MrGowdy
Copy link

MrGowdy commented Aug 10, 2021

Thank you so much @Overthane. This solved the issue for me. I managed to get myself quite lost within the node_modules...

@bleuscyther
Copy link

It seems to be because of the way Stencil State Tunnel shares state values across the hierarchy.

They would normally use events (with reserved names) to do that, but I am guessing that would be out of the scope of Stencil, at least in terms of the spirit behind it. That could be the reason why they moved it to the community repo, to separate the concerns: build cross-library components vs building a fully functional app/website.

Route

ActiveRouter.injectProps(Route, [
'location',
'history',
'historyType',
'routeViewsUpdated'
]);

Route-Link

ActiveRouter.injectProps(RouteLink, [
'history',
'location',
'root'
]);

Switch

ActiveRouter.injectProps(RouteSwitch, [
'location',
'routeViewsUpdated'
]);

Redirect

ActiveRouter.injectProps(Redirect, [
'history',
'root'
]);

I switched to the History API (1) (2)to handle the routing logic 😅. It feels less like using React or Angular

@safaalai
Copy link

safaalai commented Mar 4, 2023

An addition to @Overthane's answer:

I use cloudflare pages where when I push my code to Github, the server kicks in, does an install, pulls the code and builds everything. Of course, that means that all my changes in node_modules are lost.

So based on this StackOverflow answer I do the following:

  1. Make the changes as suggested by @Overthane
  2. Run npm install patch-package --save-dev
  3. run npx patch-package @stencil/router
  4. check in to git the patch file created in the <my-project>/patches directory
  5. Add the line "postinstall": "npx patch-package" to the script section of package.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants