Skip to content

Commit

Permalink
fix(): show warning when immutable props change
Browse files Browse the repository at this point in the history
* fix(): show warning when immutable props change (fixes #2433)

* test(): add tests for immutable prop warning
  • Loading branch information
claviska committed Jan 5, 2021
1 parent 67e0ea8 commit 9c18fa0
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 13 deletions.
26 changes: 16 additions & 10 deletions src/runtime/proxy-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type * as d from '../declarations';
import { BUILD } from '@app-data';
import { consoleDevWarn, getHostRef, plt } from '@platform';
import { getValue, setValue } from './set-value';
import { MEMBER_FLAGS } from '../utils/constants';
import { HOST_FLAGS, MEMBER_FLAGS } from '../utils/constants';
import { PROXY_FLAGS } from './runtime-constants';

export const proxyComponent = (Cstr: d.ComponentConstructor, cmpMeta: d.ComponentRuntimeMeta, flags: number) => {
Expand All @@ -23,15 +23,21 @@ export const proxyComponent = (Cstr: d.ComponentConstructor, cmpMeta: d.Componen
return getValue(this, memberName);
},
set(this: d.RuntimeRef, newValue) {
if (
// only during dev time
BUILD.isDev &&
// we are proxing the instance (not element)
(flags & PROXY_FLAGS.isElementConstructor) === 0 &&
// the member is a non-mutable prop
(memberFlags & (MEMBER_FLAGS.Prop | MEMBER_FLAGS.Mutable)) === MEMBER_FLAGS.Prop
) {
consoleDevWarn(`@Prop() "${memberName}" on "${cmpMeta.$tagName$}" cannot be modified.\nFurther information: https://stenciljs.com/docs/properties#prop-mutability`);
// only during dev time
if (BUILD.isDev) {
const ref = getHostRef(this);
if (
// we are proxying the instance (not element)
(flags & PROXY_FLAGS.isElementConstructor) === 0 &&
// the element is not constructing
(ref.$flags$ & HOST_FLAGS.isConstructingInstance) === 0 &&
// the member is a prop
(memberFlags & MEMBER_FLAGS.Prop) !== 0 &&
// the member is not mutable
(memberFlags & MEMBER_FLAGS.Mutable) === 0
) {
consoleDevWarn(`@Prop() "${memberName}" on <${cmpMeta.$tagName$}> is immutable but was modified from within the component.\nMore information: https://stenciljs.com/docs/properties#prop-mutability`);
}
}
// proxyComponent, set value
setValue(this, memberName, newValue, cmpMeta);
Expand Down
92 changes: 92 additions & 0 deletions src/runtime/test/prop-warnings.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import { Component, Prop, Method, h } from '@stencil/core';
import { newSpecPage } from '@stencil/core/testing';

describe('prop', () => {
const spy = jest.spyOn(console, 'warn').mockImplementation();

afterEach(() => spy.mockReset());
afterAll(() => spy.mockRestore());

it('should show warning when immutable prop is mutated', async () => {
@Component({ tag: 'cmp-a' })
class CmpA {
@Prop() a = 1;

@Method()
async update() {
this.a = 2;
}

render() {
return `${this.a}`;
}
}

const { root, waitForChanges } = await newSpecPage({
components: [CmpA],
html: `<cmp-a></cmp-a>`,
});

expect(root).toEqualHtml('<cmp-a>1</cmp-a>');

await root.update();
await waitForChanges();

expect(root).toEqualHtml('<cmp-a>2</cmp-a>');
expect(spy).toHaveBeenCalledTimes(1);
expect(spy.mock.calls[0][0]).toMatch(/@Prop\(\) "[A-Za-z-]+" on <[A-Za-z-]+> is immutable/);
});

it('should not show warning when mutable prop is mutated', async () => {
@Component({ tag: 'cmp-a' })
class CmpA {
@Prop({ mutable: true }) a = 1;

@Method()
async update() {
this.a = 2;
}

render() {
return `${this.a}`;
}
}

const { root, waitForChanges } = await newSpecPage({
components: [CmpA],
html: `<cmp-a></cmp-a>`,
});

expect(root).toEqualHtml('<cmp-a>1</cmp-a>');

await root.update();
await waitForChanges();

expect(root).toEqualHtml('<cmp-a>2</cmp-a>');
expect(spy).not.toHaveBeenCalled();
});

it('should not show warning when immutable prop is mutated from parent', async () => {
@Component({ tag: 'cmp-a' })
class CmpA {
@Prop() a = 1;

render() {
return `${this.a}`;
}
}

const { root, waitForChanges } = await newSpecPage({
components: [CmpA],
html: `<cmp-a></cmp-a>`,
});

expect(root).toEqualHtml('<cmp-a>1</cmp-a>');

root.a = 2;
await waitForChanges();

expect(root).toEqualHtml('<cmp-a>2</cmp-a>');
expect(spy).not.toHaveBeenCalled();
});
});
7 changes: 4 additions & 3 deletions src/testing/platform/testing-log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ export const consoleDevError = (...e: any[]) => {
caughtErrors.push(new Error(e.join(', ')));
};

export const consoleDevWarn = (..._: any[]) => {
/* noop for testing */
export const consoleDevWarn = (args: any) => {
// log warnings so we can spy on them when testing
console.warn(args);
};

export const consoleDevInfo = (..._: any[]) => {
/* noop for testing */
};

export const setErrorHandler = (handler: d.ErrorHandler) => customError = handler;
export const setErrorHandler = (handler: d.ErrorHandler) => (customError = handler);

0 comments on commit 9c18fa0

Please sign in to comment.