From a75b9a717c237830e55147de898269dc205728d3 Mon Sep 17 00:00:00 2001 From: Rongyan Chen Date: Wed, 20 Apr 2022 02:18:39 -0700 Subject: [PATCH] feat: remove freeze and add some test cases (#53) * feat: remove freeze and add some test cases * fix: rename isObject > isPlainObject * fix: export toRaw --- .../elements/__tests__/HTMLElement.test.js | 8 +- packages/pwc/src/index.ts | 2 +- .../src/reactivity/__tests__/reactive.test.ts | 104 +++++++++--------- packages/pwc/src/reactivity/reactive.ts | 8 +- packages/pwc/src/utils/checkTypes.ts | 2 +- packages/pwc/src/utils/index.ts | 2 +- packages/pwc/src/utils/shallowClone.ts | 23 ++++ .../pwc/src/utils/shallowCloneAndFreeze.ts | 28 ----- 8 files changed, 86 insertions(+), 91 deletions(-) create mode 100644 packages/pwc/src/utils/shallowClone.ts delete mode 100644 packages/pwc/src/utils/shallowCloneAndFreeze.ts diff --git a/packages/pwc/src/elements/__tests__/HTMLElement.test.js b/packages/pwc/src/elements/__tests__/HTMLElement.test.js index e35cb35..365b74f 100644 --- a/packages/pwc/src/elements/__tests__/HTMLElement.test.js +++ b/packages/pwc/src/elements/__tests__/HTMLElement.test.js @@ -17,7 +17,7 @@ function getSimpleCustomElement() { [ { name: 'onclick', - value: this.onClick.bind(this), + value: this.onClick, } ], this.text, @@ -44,7 +44,7 @@ function getNestedCustomElement() { [ { name: 'onclick', - value: this.onClick.bind(this), + value: this.onClick, } ], this.text, @@ -85,7 +85,7 @@ function getReactiveCustomElement() { }, { name: 'onclick', - value: this.onClick.bind(this), + value: this.onClick, } ], this.#text, @@ -185,6 +185,8 @@ describe('Render nested components', () => { items = []; + callback() {} + get template() { return mockChildFn(this); } diff --git a/packages/pwc/src/index.ts b/packages/pwc/src/index.ts index f65daca..a48b82f 100644 --- a/packages/pwc/src/index.ts +++ b/packages/pwc/src/index.ts @@ -2,6 +2,6 @@ import './elements'; export { nextTick } from './elements/sheduler'; export * from './decorators'; -export { toRaw } from './utils/toRaw'; +export { toRaw } from './utils'; export { compileTemplateInRuntime as html } from '@pwc/compiler/compileTemplateInRuntime'; diff --git a/packages/pwc/src/reactivity/__tests__/reactive.test.ts b/packages/pwc/src/reactivity/__tests__/reactive.test.ts index c377176..920fa56 100644 --- a/packages/pwc/src/reactivity/__tests__/reactive.test.ts +++ b/packages/pwc/src/reactivity/__tests__/reactive.test.ts @@ -1,4 +1,5 @@ import { Reactive } from '../reactive'; +import { toRaw } from '../../utils'; class MockReactiveElement { #initialized = false; @@ -9,6 +10,7 @@ class MockReactiveElement { }; constructor(initialValue) { this.reactive.initValue('#data', initialValue); + this.reactive.initValue('data', initialValue); this.#initialized = true; this.isUpdating = false; } @@ -18,26 +20,10 @@ class MockReactiveElement { get data() { return this.reactive.getValue('#data'); } - _requestUpdate() { - this.isUpdating = true; - } -} -class MockNotReactiveElement { - #initialized = false; - isUpdating = false; - reactive = new Reactive(this); - _getInitialState() { - return this.#initialized; - }; - constructor(initialValue) { - this.reactive.initValue('data', initialValue); - this.#initialized = true; - this.isUpdating = false; - } - set data(val) { + set publicData(val) { this.reactive.setValue('data', val); } - get data() { + get publicData() { return this.reactive.getValue('data'); } _requestUpdate() { @@ -49,9 +35,19 @@ describe('Create a reactive property', () => { it('A primitive property should request a update', () => { const element = new MockReactiveElement('Jack'); expect(element.isUpdating).toBe(false); + + // private property element.data = 'Tom'; expect(element.isUpdating).toBe(true); expect(element.data).toBe('Tom'); + + element.isUpdating = false; + expect(element.isUpdating).toBe(false); + + // public property + element.publicData = 'Tom'; + expect(element.isUpdating).toBe(true); + expect(element.publicData).toBe('Tom'); }); it('A object property should request a update', () => { const element = new MockReactiveElement({ @@ -59,6 +55,7 @@ describe('Create a reactive property', () => { }); expect(element.isUpdating).toBe(false); + // 1. private property // change value element.data.name = 'Tom'; expect(element.isUpdating).toBe(true); @@ -75,11 +72,30 @@ describe('Create a reactive property', () => { delete element.data['number']; expect(element.isUpdating).toBe(true); expect(element.data).toEqual({ name: 'Tom' }); + + // 2. public property + // change value + element.publicData.name = 'Tom'; + expect(element.isUpdating).toBe(true); + expect(element.publicData).toEqual({ name: 'Tom' }); + element.isUpdating = false; + + // add prop + element.publicData.number = '1'; + expect(element.isUpdating).toBe(true); + expect(element.publicData).toEqual({ name: 'Tom', number: '1' }); + element.isUpdating = false; + + // delete prop + delete element.publicData['number']; + expect(element.isUpdating).toBe(true); + expect(element.publicData).toEqual({ name: 'Tom' }); }); it('A array property should request a update', () => { const element = new MockReactiveElement(['Jack']); expect(element.isUpdating).toBe(false); + // 1. private property // push element.data.push('Tom') expect(element.isUpdating).toBe(true); @@ -90,44 +106,26 @@ describe('Create a reactive property', () => { element.data.splice(1, 1); expect(element.isUpdating).toBe(true); expect(element.data).toEqual(['Jack']); - }); -}); - -describe('Create a normal property', () => { - function assertError(it: any, msg: string) { - expect(it).toThrow(TypeError); - expect(it).toThrow(msg); - } - it('It should request a update when the source changed', () => { - const element = new MockNotReactiveElement('Jack'); - expect(element.isUpdating).toBe(false); - element.data = 'Tom'; + // 2. public property + // push + element.publicData.push('Tom') expect(element.isUpdating).toBe(true); - expect(element.data).toBe('Tom'); - }); - - it('It should throw a error when a readonly object changed', () => { - const element = new MockNotReactiveElement({ - name: 'Jack' - }); - // change value - assertError(() => element.data.name = 'Tom', `Cannot assign to read only property 'name' of object '#'`); - - // add prop - assertError(() => element.data.number = '1', 'Cannot add property number, object is not extensible'); - - // delete prop - assertError(() => delete element.data['name'], `Cannot delete property 'name' of #`); - }); - - it('It should not request a update when a item changed', () => { - const element = new MockNotReactiveElement(['Jack']); + expect(element.publicData).toEqual(['Jack', 'Tom']); + element.isUpdating = false; - // push - assertError(() => element.data.push('Tom'), 'Cannot add property 1, object is not extensible'); - // splice - assertError(() => element.data.splice(1, 1), `Cannot assign to read only property 'length' of object '[object Array]'`); + element.publicData.splice(1, 1); + expect(element.isUpdating).toBe(true); + expect(element.publicData).toEqual(['Jack']); }); + it('Public Data Should be shallow cloned', () => { + const data = { + someObject: {} + }; + const element = new MockReactiveElement(data); + + expect(toRaw(element.data) === data).toBe(true); + expect(toRaw(element.publicData) === data).toBe(false); + }) }); diff --git a/packages/pwc/src/reactivity/reactive.ts b/packages/pwc/src/reactivity/reactive.ts index 4878184..60f3d00 100644 --- a/packages/pwc/src/reactivity/reactive.ts +++ b/packages/pwc/src/reactivity/reactive.ts @@ -1,4 +1,4 @@ -import { isArray, isObject, isPrivate, shallowCloneAndFreeze } from '../utils'; +import { isArray, isPlainObject, isPrivate, shallowClone } from '../utils'; import { getProxyHandler } from './handler'; interface ReactiveType { @@ -41,8 +41,8 @@ export class Reactive implements ReactiveType { if (isPrivate(prop)) { this.#setReactiveValue(prop, value); } else { - // Clone and Freeze public props and it should not be reactive - this.#setNormalValue(prop, shallowCloneAndFreeze(value)); + // It should shallow clone public props to prevent effects of passing by reference + this.#setReactiveValue(prop, shallowClone(value)); } if (forceUpdate) { @@ -51,7 +51,7 @@ export class Reactive implements ReactiveType { } #setReactiveValue(prop: string, value: unknown) { - if (isArray(value) || isObject(value)) { + if (isArray(value) || isPlainObject(value)) { this.#createReactiveProperty(prop, value); } else { this.#setNormalValue(prop, value); diff --git a/packages/pwc/src/utils/checkTypes.ts b/packages/pwc/src/utils/checkTypes.ts index f5906de..bcbe9d1 100644 --- a/packages/pwc/src/utils/checkTypes.ts +++ b/packages/pwc/src/utils/checkTypes.ts @@ -17,7 +17,7 @@ export function isFunction(value: unknown) { return typeof value === 'function'; } -export function isObject(value: unknown) { +export function isPlainObject(value: unknown) { return Object.prototype.toString.call(value) === '[object Object]'; } diff --git a/packages/pwc/src/utils/index.ts b/packages/pwc/src/utils/index.ts index 506e082..67bf3b9 100644 --- a/packages/pwc/src/utils/index.ts +++ b/packages/pwc/src/utils/index.ts @@ -3,6 +3,6 @@ export * from './isEventName'; export * from './shallowEqual'; export * from './generateUid'; export * from './checkTypes'; -export * from './shallowCloneAndFreeze'; +export * from './shallowClone'; export * from './toRaw'; diff --git a/packages/pwc/src/utils/shallowClone.ts b/packages/pwc/src/utils/shallowClone.ts new file mode 100644 index 0000000..f16bd34 --- /dev/null +++ b/packages/pwc/src/utils/shallowClone.ts @@ -0,0 +1,23 @@ +import { isArray, isPlainObject, isMap, isSet } from './checkTypes'; + +// Shallow Clone the Property value +// Attention, it only clones Set\Map\Array\Plain Object +export function shallowClone(value: any) { + if (isSet(value)) { + return new Set(value); + } + + if (isMap(value)) { + return new Map(value); + } + + if (isArray(value)) { + return [...value]; + } + + if (isPlainObject(value)) { + return Object.assign({}, value); + } + + return value; +} diff --git a/packages/pwc/src/utils/shallowCloneAndFreeze.ts b/packages/pwc/src/utils/shallowCloneAndFreeze.ts deleted file mode 100644 index 2749a9c..0000000 --- a/packages/pwc/src/utils/shallowCloneAndFreeze.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { isArray, isObject, isMap, isSet } from './checkTypes'; - -// Shallow Clone the Property value and Freeze it -// Attention, it only clones Set\Map\Array\Object([Object, Object]) -// and freezes Array\Object props -export function shallowCloneAndFreeze(value: any) { - if (isSet(value)) { - return new Set(value); - } - - if (isMap(value)) { - return new Map(value); - } - - if (isArray(value)) { - const props = [...value]; - Object.freeze(props); - return props; - } - - if (isObject(value)) { - const props = Object.assign({}, value); - Object.freeze(props); - return props; - } - - return value; -}