Unit test changes property, warns that property was changed within the component

[Paitum]

Stencil version:

 @stencil/[email protected]

I'm submitting a:

[x] bug report
[ ] 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:

A JEST unit test changes a component's property imperatively, and StencilJS warns that a property "was modified from within the component".

Expected behavior:

Changing property values from outside a component should not warn that the property was changed from "within" the component.

Steps to reproduce:

Consider the following MyTable component. Notice that it takes an object property that must be set imperatively (via JavaScript), and that it makes no changes to this object.

Component code:

@Component({
  tag: 'my-table'
})
export class MyTable {
  @Prop() data: any;
}

Test code:

describe('my-table', () => {
  it('processes data changes', async () => {
    const page = await newSpecPage({
      components: [MyTable],
      html: '<my-table/>'
    });

    // expect table behaves correctly without data

    const table = page.rootInstance;
    table.data = MOCK_DATA;

    await page.waitForChanges();

    // expect table behaves correctly with data
  });
});

Console output:

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

      32 |
      33 |     table = page.rootInstance;
    > 34 |     table.data = columns;
         |               ^
      35 |
      36 |     await page.waitForChanges();
      37 |

Other information:

The test succeeds, as this is just a warning, but the output will flood CI job logs and create red-herrings for unfamiliar developers.

[NewDark90]

Yep, getting the same issue.

I don't like it but the temporary solution might just be to override console.warn and swallow these specific warnings. I'd be ideal if the warnings only showed up after the element was attached.

[NewDark90]

Really simple implementation to swallow these specific warnings for anyone that might want a starter without thinking about it.

const consoleWarnFn = console.warn;

beforeEach(async () => {
    console.warn = (...args: any[]) => {
        const arg1 = args[0];
        if (typeof arg1 === "string" && arg1.includes("stenciljs.com/docs/properties#prop-mutability"))
            return undefined;

        consoleWarnFn(...args);
    }
})

afterEach(() => {
    console.warn = consoleWarnFn;
});

[littleninja]

The warnings occur because the test mutates a non-mutable property. The developer could adjust either the property decorator or test setup, depending on the right outcome for their use case:

For example, initialize the property from the attribute in the setup:

describe('my-table', () => {
  it('processes data changes', async () => {
    const page = await newSpecPage({
      components: [MyTable],
      html: `<my-table data="${MOCK_DATA}" />`
    });
    
    // ... continue with test
    
    // and I haven't tried this out but could .setAttribute() work?
    const tableElement = page.doc.querySelector('my-table');
    tableElement.setAttribute('data', NEW_MOCK_DATA);
    
    await page.waitForChanges();
    // ...  and continue with test
  });
});

If the property should be mutable from inside the component (beyond the unit tests):

@Component({
  tag: 'my-table'
})
export class MyTable {
  @Prop({ mutable: true }) data: any;
}

[NewDark90]

@littleninja Those solutions for setting the attribute might work for simpler properties, numbers/strings/booleans/etc. However, they won't work with objects or functions.

[splitinfinities]

I think that broadly we think that these developer-centric notifications should be reduced from a console.warn to a console.debug when doing this, we may also be able to check if the context of the program is run within Jest, so we could use that to disable the console.

This would also be a great first issue for folks to pick up and run with!

[amylo]

Hi, I would like to make a pull request to resolve this issue.

I'm thinking of providing a flag in Stencil testing config:

import { Config } from '@stencil/core';
export const config: Config = {
  testing: {
    compilerLogLevel: 'error'     // shows only console.error from compiler (dev mode)
  }
};

If people want to see these compiler console warnings, they set compilerLogLevel: 'warn':

export const config: Config = {
  testing: {
    compilerLogLevel: 'warn'     // shows console.warn AND console.error from compiler (dev mode)
  }
};

And since most people don't want to see these warnings anyways, I think the default value (if compilerLogLevel is not provided in their testing config) should be 'error' so only compiler errors will be appear in the logs. This way everyone who updates to the new version of Stencil, will get this issue fixed for them automatically.

[dvag-sebastian-goeb]

I feel like swallowing the compiler warning via mocked console.warn or test log levels are both just workarounds. I'm not too familiar with the internals of how a spec page is run, but I would expect statements run inside the test function to occur OUTSIDE of the component. It seems strange to me that the it('should ...', () => { page.rootInstance.prop = object; }) callback function would be executed from within the component. Why is that, anyway?

[Bibli2311]

Using ".setAttribute()" worked for me to avoid getting console.warn() messages.
I think this is better solution then swallowing Stencil specific messages in console.warning for properties with simple data variables (numbers/ booleans/ strings etc.)

#2832 (comment)

[MichaelHolley]

Using ".setAttribute()" worked for me to avoid getting console.warn() messages. I think this is better solution then swallowing Stencil specific messages in console.warning for properties with simple data variables (numbers/ booleans/ strings etc.)

#2832 (comment)

setAttribute()
and
await page.waitForChanges(); did the trick for me. thx

[christian-bromann]

Hey folks 👋 I just took a look into this and it appears that I can't reproduce this anymore. I tested on a component starter using the following test:

  it('renders with values', async () => {
    const { waitForChanges, doc, root } = await newSpecPage({
      components: [MyComponent],
      template: () => (
        <my-component getText={() => 'someone'} first="haha!!" last="'Don't call me a framework' JS"></my-component>
      ),
    });

    await waitForChanges()
    doc.querySelector('my-component').getText = () => 'someone else';
    await waitForChanges()

    expect(root).toEqualHtml(`
      <my-component>
        <mock:shadow-root>
          <div>
            Hello, World! I'm someone else
          </div>
        </mock:shadow-root>
      </my-component>
    `);
  });

with the following changes to the component:

diff --git a/src/components/my-component/my-component.tsx b/src/components/my-component/my-component.tsx
index 56d51d9..3ce0825 100644
--- a/src/components/my-component/my-component.tsx
+++ b/src/components/my-component/my-component.tsx
@@ -22,9 +22,7 @@ export class MyComponent {
    */
   @Prop() last: string;

-  private getText(): string {
-    return format(this.first, this.middle, this.last);
-  }
+  @Prop({ mutable: false }) getText: () => string

   render() {
     return <div>Hello, World! I'm {this.getText()}</div>;

I will go ahead and close the issue. I am happy to re-open if a reproducible example can be provided. Thank you all!