From db4ab8b10872ff1ca492c02ad0f62368ed748631 Mon Sep 17 00:00:00 2001 From: Illya Klymov Date: Tue, 5 Jan 2021 07:59:13 +0200 Subject: [PATCH] fix: prevent setProps infinite loop with immediate watchers (#1752) * prevent `setProps` from being called on non-top level wrappers * remove useless `silent` option from config --- docs/api/config.md | 17 +--- docs/api/wrapper/setProps.md | 4 +- docs/ja/api/config.md | 15 --- docs/ru/api/config.md | 15 --- docs/ru/api/wrapper/setProps.md | 4 +- docs/zh/api/config.md | 17 +--- flow/config.flow.js | 1 - packages/create-instance/create-instance.js | 18 +++- packages/server-test-utils/types/index.d.ts | 1 - .../types/test/renderToString.ts | 1 - packages/test-utils/src/config.js | 1 - packages/test-utils/src/wrapper.js | 98 +++++++------------ packages/test-utils/types/index.d.ts | 3 +- packages/test-utils/types/test/mount.ts | 3 +- .../component-with-watch-immediate.vue | 24 +++++ test/specs/config.spec.js | 38 ------- test/specs/wrapper/props.spec.js | 4 +- test/specs/wrapper/setProps.spec.js | 34 ++++++- 18 files changed, 120 insertions(+), 178 deletions(-) create mode 100644 test/resources/components/component-with-watch-immediate.vue diff --git a/docs/api/config.md b/docs/api/config.md index a737b80c5..5a84aa259 100644 --- a/docs/api/config.md +++ b/docs/api/config.md @@ -47,7 +47,7 @@ config.deprecationWarningHandler = (method, message) => { - type: `{ [name: string]: Component | boolean | string }` - default: `{}` -The stub stored in `config.stubs` is used by default. +The stub stored in `config.stubs` is used by default. Stubs to use in components. These are overwritten by `stubs` passed in the mounting options. When passing `stubs` as an array in the mounting options, `config.stubs` are converted to an array, and will stub components with a basic component that returns `<${component name}-stub>`. @@ -112,18 +112,3 @@ config.provide['$logger'] = { } } ``` - -### `silent` - -- type: `Boolean` -- default: `true` - -It suppresses warnings triggered by Vue while mutating component's observables (e.g. props). When set to `false`, all warnings are visible in the console. This is a configurable way which relies on `Vue.config.silent`. - -Example: - -```js -import { config } from '@vue/test-utils' - -config.silent = false -``` diff --git a/docs/api/wrapper/setProps.md b/docs/api/wrapper/setProps.md index 9fc39cc7d..9a86ea0e1 100644 --- a/docs/api/wrapper/setProps.md +++ b/docs/api/wrapper/setProps.md @@ -8,7 +8,9 @@ Sets `Wrapper` `vm` props and forces update. -**Note the Wrapper must contain a Vue instance.** +::: warning +`setProps` could be called only for top-level component, mounted by `mount` or `shallowMount` +::: ```js import { mount } from '@vue/test-utils' diff --git a/docs/ja/api/config.md b/docs/ja/api/config.md index e37c8e1b0..2dcad4997 100644 --- a/docs/ja/api/config.md +++ b/docs/ja/api/config.md @@ -73,18 +73,3 @@ config.provide['$logger'] = { } } ``` - -### `silent` - -- 型: `Boolean` -- デフォルト: `true` - -Vue がコンポーネントの変更を感知するプロパティ(例えば props )が変更される時に出す警告を出力しません。`false` をセットするとすべての警告はコンソールに表示されません。この機能は `Vue.config.silent` を使って実現しています。 - -例: - -```js -import { config } from '@vue/test-utils' - -config.silent = false -``` diff --git a/docs/ru/api/config.md b/docs/ru/api/config.md index f66cb6420..5500bf75b 100644 --- a/docs/ru/api/config.md +++ b/docs/ru/api/config.md @@ -74,18 +74,3 @@ config.provide['$logger'] = { } } ``` - -### `silent` - -- Тип: `Boolean` -- По умолчанию: `true` - -Подавляет предупреждения, вызванные Vue во время изменения наблюдаемых компонентов (например, входных параметров). Если установлено значение `false`, все предупреждения показываются в консоли. Это настраиваемый способ, который основывается на `Vue.config.silent`. - -Пример: - -```js -import { config } from '@vue/test-utils' - -config.silent = false -``` diff --git a/docs/ru/api/wrapper/setProps.md b/docs/ru/api/wrapper/setProps.md index 9e4764161..6cb8e5cae 100644 --- a/docs/ru/api/wrapper/setProps.md +++ b/docs/ru/api/wrapper/setProps.md @@ -8,7 +8,9 @@ Устанавливает входные параметры `Wrapper` `vm` и выполняет принудительное обновление. -**Обратите внимание, что `Wrapper` должен содержать экземпляр Vue.** +::: warning Обратите внимание! +`setProps` может быть вызван только на `wrapper` верхнего уровня, который был создан с помощью `mount` или `shallowMount` +::: ```js import { mount } from '@vue/test-utils' diff --git a/docs/zh/api/config.md b/docs/zh/api/config.md index 71a17a334..1e2aa4756 100644 --- a/docs/zh/api/config.md +++ b/docs/zh/api/config.md @@ -24,7 +24,7 @@ config.showDeprecationWarnings = false - 类型：`{ [name: string]: Component | boolean | string }` - 默认值：`{}` -存储在 `config.stubs` 中的存根会被默认使用。 +存储在 `config.stubs` 中的存根会被默认使用。 用到的组件存根。它们会被传入挂载选项的 `stubs` 覆写。 当把 `stubs` 作为一个数组传入挂载选项时，`config.stubs` 会被转换为一个数组，然后用只返回一个 `<${component name}-stub>` 的基础组件进行存根。 @@ -89,18 +89,3 @@ config.provide['$logger'] = { } } ``` - -### `silent` - -- 类型：`Boolean` -- 默认值：`true` - -在组件的可观察内容 (如 props) 发生突变时，警告会被 Vue 阻止。当设置为 `false` 时，所有的警告都会出现在控制台中。这是一个 `Vue.config.silent` 的配置方式。 - -示例； - -```js -import { config } from '@vue/test-utils' - -config.silent = false -``` diff --git a/flow/config.flow.js b/flow/config.flow.js index a53bb381d..f23909d0c 100644 --- a/flow/config.flow.js +++ b/flow/config.flow.js @@ -3,6 +3,5 @@ declare type Config = { mocks?: Object, methods?: { [name: string]: Function }, provide?: Object, - silent?: boolean, showDeprecationWarnings?: boolean } diff --git a/packages/create-instance/create-instance.js b/packages/create-instance/create-instance.js index 68b12774a..3c0ba7d2e 100644 --- a/packages/create-instance/create-instance.js +++ b/packages/create-instance/create-instance.js @@ -11,7 +11,7 @@ import createScopedSlots from './create-scoped-slots' import { createStubsFromStubsObject } from './create-component-stubs' import { patchCreateElement } from './patch-create-element' -function createContext(options, scopedSlots) { +function createContext(options, scopedSlots, currentProps) { const on = { ...(options.context && options.context.on), ...options.listeners @@ -20,8 +20,8 @@ function createContext(options, scopedSlots) { attrs: { ...options.attrs, // pass as attrs so that inheritAttrs works correctly - // propsData should take precedence over attrs - ...options.propsData + // props should take precedence over attrs + ...currentProps }, ...(options.context || {}), on, @@ -110,16 +110,26 @@ export default function createInstance( parentComponentOptions.provide = function() { return { ...getValuesFromCallableOption.call(this, originalParentComponentProvide), + // $FlowIgnore ...getValuesFromCallableOption.call(this, options.provide) } } + const originalParentComponentData = parentComponentOptions.data + parentComponentOptions.data = function() { + return { + ...getValuesFromCallableOption.call(this, originalParentComponentData), + vueTestUtils_childProps: { ...options.propsData } + } + } + parentComponentOptions.$_doNotStubChildren = true + parentComponentOptions.$_isWrapperParent = true parentComponentOptions._isFunctionalContainer = componentOptions.functional parentComponentOptions.render = function(h) { return h( Constructor, - createContext(options, scopedSlots), + createContext(options, scopedSlots, this.vueTestUtils_childProps), createChildren(this, h, options) ) } diff --git a/packages/server-test-utils/types/index.d.ts b/packages/server-test-utils/types/index.d.ts index 6eacf3530..4319edd58 100644 --- a/packages/server-test-utils/types/index.d.ts +++ b/packages/server-test-utils/types/index.d.ts @@ -45,7 +45,6 @@ interface VueTestUtilsConfigOptions { mocks?: object methods?: Record provide?: object, - silent?: Boolean } export declare let config: VueTestUtilsConfigOptions diff --git a/packages/server-test-utils/types/test/renderToString.ts b/packages/server-test-utils/types/test/renderToString.ts index 8e5e70c94..e6cee9df2 100644 --- a/packages/server-test-utils/types/test/renderToString.ts +++ b/packages/server-test-utils/types/test/renderToString.ts @@ -66,4 +66,3 @@ config.methods = { config.provide = { foo: {} } -config.silent = true diff --git a/packages/test-utils/src/config.js b/packages/test-utils/src/config.js index 680e92b2a..444e23104 100644 --- a/packages/test-utils/src/config.js +++ b/packages/test-utils/src/config.js @@ -6,7 +6,6 @@ export default { mocks: {}, methods: {}, provide: {}, - silent: true, showDeprecationWarnings: typeof process.env.SHOW_DEPRECATIONS !== 'undefined' ? process.env.SHOW_DEPRECATIONS diff --git a/packages/test-utils/src/wrapper.js b/packages/test-utils/src/wrapper.js index 3b3160949..cf127f496 100644 --- a/packages/test-utils/src/wrapper.js +++ b/packages/test-utils/src/wrapper.js @@ -1,6 +1,5 @@ // @flow -import Vue from 'vue' import pretty from 'pretty' import getSelector from './get-selector' import { @@ -9,7 +8,6 @@ import { VUE_VERSION, DOM_SELECTOR } from 'shared/consts' -import config from './config' import WrapperArray from './wrapper-array' import ErrorWrapper from './error-wrapper' import { @@ -721,71 +719,49 @@ export default class Wrapper implements BaseWrapper { if (!this.vm) { throwError(`wrapper.setProps() can only be called on a Vue instance`) } - this.__warnIfDestroyed() - // Save the original "silent" config so that we can directly mutate props - const originalConfig = Vue.config.silent - Vue.config.silent = config.silent - - try { - Object.keys(data).forEach(key => { - // Don't let people set entire objects, because reactivity won't work - if ( - typeof data[key] === 'object' && - data[key] !== null && - // $FlowIgnore : Problem with possibly null this.vm - data[key] === this.vm[key] - ) { - throwError( - `wrapper.setProps() called with the same object of the existing ` + - `${key} property. You must call wrapper.setProps() with a new ` + - `object to trigger reactivity` - ) - } + // $FlowIgnore : Problem with possibly null this.vm + if (!this.vm.$parent.$options.$_isWrapperParent) { + throwError( + `wrapper.setProps() can only be called for top-level component` + ) + } - if ( - !this.vm || - !this.vm.$options._propKeys || - !this.vm.$options._propKeys.some(prop => prop === key) - ) { - if (VUE_VERSION > 2.3) { - // $FlowIgnore : Problem with possibly null this.vm - this.vm.$attrs[key] = data[key] - return nextTick() - } - throwError( - `wrapper.setProps() called with ${key} property which ` + - `is not defined on the component` - ) - } + this.__warnIfDestroyed() - // Actually set the prop + Object.keys(data).forEach(key => { + // Don't let people set entire objects, because reactivity won't work + if ( + typeof data[key] === 'object' && + data[key] !== null && // $FlowIgnore : Problem with possibly null this.vm - this.vm[key] = data[key] - }) + data[key] === this.vm[key] + ) { + throwError( + `wrapper.setProps() called with the same object of the existing ` + + `${key} property. You must call wrapper.setProps() with a new ` + + `object to trigger reactivity` + ) + } + + if ( + VUE_VERSION <= 2.3 && + (!this.vm || + !this.vm.$options._propKeys || + !this.vm.$options._propKeys.some(prop => prop === key)) + ) { + throwError( + `wrapper.setProps() called with ${key} property which ` + + `is not defined on the component` + ) + } // $FlowIgnore : Problem with possibly null this.vm - this.vm.$forceUpdate() - return new Promise(resolve => { - nextTick().then(() => { - const isUpdated = Object.keys(data).some(key => { - return ( - // $FlowIgnore : Problem with possibly null this.vm - this.vm[key] === data[key] || - // $FlowIgnore : Problem with possibly null this.vm - (this.vm.$attrs && this.vm.$attrs[key] === data[key]) - ) - }) - return !isUpdated ? this.setProps(data).then(resolve()) : resolve() - }) - }) - } catch (err) { - throw err - } finally { - // Ensure you teardown the modifications you made to the user's config - // After all the props are set, then reset the state - Vue.config.silent = originalConfig - } + const parent = this.vm.$parent + parent.$set(parent.vueTestUtils_childProps, key, data[key]) + }) + + return nextTick() } /** diff --git a/packages/test-utils/types/index.d.ts b/packages/test-utils/types/index.d.ts index 7e7c197b3..05b78fbca 100644 --- a/packages/test-utils/types/index.d.ts +++ b/packages/test-utils/types/index.d.ts @@ -85,7 +85,7 @@ export interface Wrapper extends BaseWrapper { get (selector: string): Wrapper get (selector: RefSelector): Wrapper get (selector: NameSelector): Wrapper - + getComponent (selector: VueClass): Wrapper getComponent (selector: ComponentOptions): Wrapper getComponent>(selector: FunctionalComponentOptions): Wrapper @@ -170,7 +170,6 @@ interface VueTestUtilsConfigOptions { mocks: Record methods: Record provide?: Record, - silent?: Boolean, showDeprecationWarnings?: boolean deprecationWarningHandler?: Function } diff --git a/packages/test-utils/types/test/mount.ts b/packages/test-utils/types/test/mount.ts index 4a92c0ca9..038b08205 100644 --- a/packages/test-utils/types/test/mount.ts +++ b/packages/test-utils/types/test/mount.ts @@ -101,8 +101,7 @@ config.provide = { config.provide['foo'] = { bar: {} } -config.silent = true config.showDeprecationWarnings = false // Check we can use default export -VueTestUtils.config.silent = false +VueTestUtils.config.showDeprecationWarnings = false diff --git a/test/resources/components/component-with-watch-immediate.vue b/test/resources/components/component-with-watch-immediate.vue new file mode 100644 index 000000000..f8abce285 --- /dev/null +++ b/test/resources/components/component-with-watch-immediate.vue @@ -0,0 +1,24 @@ + + + diff --git a/test/specs/config.spec.js b/test/specs/config.spec.js index e77d31f6d..761a9d66d 100644 --- a/test/specs/config.spec.js +++ b/test/specs/config.spec.js @@ -1,23 +1,19 @@ import { describeWithShallowAndMount } from '~resources/utils' -import ComponentWithProps from '~resources/components/component-with-props.vue' import { config, createLocalVue } from 'packages/test-utils/src' import ComponentWithTransitions from '~resources/components/component-with-transitions.vue' describeWithShallowAndMount('config', mountingMethod => { let configStubsSave - let configSilentSave let consoleErrorSave beforeEach(() => { configStubsSave = config.stubs - configSilentSave = config.silent consoleErrorSave = console.error console.error = jest.fn() }) afterEach(() => { config.stubs = configStubsSave - config.silent = configSilentSave config.methods = {} config.deprecationWarningHandler = null console.error = consoleErrorSave @@ -62,40 +58,6 @@ describeWithShallowAndMount('config', mountingMethod => { expect(wrapper.text()).toEqual('method') }) - it("doesn't throw Vue warning when silent is set to true", () => { - config.silent = true - const localVue = createLocalVue() - const wrapper = mountingMethod(ComponentWithProps, { - propsData: { - prop1: 'example' - }, - localVue - }) - expect(wrapper.vm.prop1).toEqual('example') - wrapper.setProps({ - prop1: 'new value' - }) - expect(console.error).not.toHaveBeenCalled() - }) - - it('does throw Vue warning when silent is set to false', () => { - config.silent = false - const localVue = createLocalVue() - const wrapper = mountingMethod(ComponentWithProps, { - propsData: { - prop1: 'example' - }, - localVue - }) - expect(wrapper.vm.prop1).toEqual('example') - wrapper.setProps({ - prop1: 'new value' - }) - expect(console.error).toHaveBeenCalledWith( - expect.stringMatching(/[Vue warn]/) - ) - }) - it('stubs out transitions by default', async () => { const wrapper = mountingMethod(ComponentWithTransitions) expect(wrapper.find('[data-testid="expanded"]').exists()).toEqual(true) diff --git a/test/specs/wrapper/props.spec.js b/test/specs/wrapper/props.spec.js index 3da011614..41e9d47cb 100644 --- a/test/specs/wrapper/props.spec.js +++ b/test/specs/wrapper/props.spec.js @@ -22,7 +22,7 @@ describeWithShallowAndMount('props', mountingMethod => { expect(wrapper.props()).toEqual({}) }) - it('should update after setProps', () => { + it('should update after setProps', async () => { const prop1 = {} const prop2 = 'val1' const wrapper = mountingMethod(ComponentWithProps, { @@ -31,7 +31,7 @@ describeWithShallowAndMount('props', mountingMethod => { expect(wrapper.props()).toEqual({ prop1: {}, prop2: 'val1' }) // setProps - wrapper.setProps({ prop2: 'val2' }) + await wrapper.setProps({ prop2: 'val2' }) expect(wrapper.vm.prop2).toEqual('val2') // pass expect(wrapper.props()).toEqual({ prop1: {}, prop2: 'val2' }) // fail }) diff --git a/test/specs/wrapper/setProps.spec.js b/test/specs/wrapper/setProps.spec.js index 039be14a8..5f753be47 100644 --- a/test/specs/wrapper/setProps.spec.js +++ b/test/specs/wrapper/setProps.spec.js @@ -1,6 +1,8 @@ import { compileToFunctions } from 'vue-template-compiler' +import ComponentWithChild from '~resources/components/component-with-child.vue' import ComponentWithProps from '~resources/components/component-with-props.vue' import ComponentWithWatch from '~resources/components/component-with-watch.vue' +import ComponentWithWatchImmediate from '~resources/components/component-with-watch-immediate.vue' import { describeWithShallowAndMount, isPromise, @@ -260,6 +262,29 @@ describeWithShallowAndMount('setProps', mountingMethod => { expect(callback).toHaveBeenCalledTimes(2) } ) + + it('correctly handles consecutive setProps calls when immediate watcher is attached', async () => { + const wrapper = mountingMethod(ComponentWithWatchImmediate, { + propsData: { prop1: 'zero' } + }) + + await wrapper.setProps({ prop1: 'one' }) + await wrapper.setProps({ prop1: 'two' }) + + expect(wrapper.vm.prop1).toBe('two') + }) + + it('correctly handles multiple setProps calls in one tick when immediate watcher is attached', async () => { + const wrapper = mountingMethod(ComponentWithWatchImmediate, { + propsData: { prop1: 'zero' } + }) + + wrapper.setProps({ prop1: 'one' }) + wrapper.setProps({ prop1: 'two' }) + await wrapper.vm.$nextTick() + + expect(wrapper.vm.prop1).toBe('two') + }) }) it('props and setProps should return the same reference when called with same object', async () => { @@ -278,7 +303,8 @@ describeWithShallowAndMount('setProps', mountingMethod => { FUNCTIONAL_COMPONENT_ERROR: `[vue-test-utils]: wrapper.setProps() cannot be called on a functional component`, SAME_REFERENCE_ERROR: `[vue-test-utils]: wrapper.setProps() called with the same object of the existing obj property. You must call wrapper.setProps() with a new object to trigger reactivity`, INVALID_NODE_ERROR: `wrapper.setProps() can only be called on a Vue instance`, - WRONG_PROP_ERROR: `[vue-test-utils]: wrapper.setProps() called with prop1 property which is not defined on the component` + WRONG_PROP_ERROR: `[vue-test-utils]: wrapper.setProps() called with prop1 property which is not defined on the component`, + ONLY_TOP_LEVEL_ERROR: `[vue-test-utils]: wrapper.setProps() can only be called for top-level component` } describe('functional components', () => { @@ -347,5 +373,11 @@ describeWithShallowAndMount('setProps', mountingMethod => { errors.INVALID_NODE_ERROR ) }) + + it('throws an error if called on child component', () => { + const wrapper = mountingMethod(ComponentWithChild) + const child = wrapper.find({ ref: 'child' }) + expect(() => child.setProps({})).toThrow(errors.ONLY_TOP_LEVEL_ERROR) + }) }) })