From b939a01a4a22e28315e4ebdf0c8be0269039606d Mon Sep 17 00:00:00 2001 From: Erik Rasmussen Date: Tue, 23 Aug 2016 08:04:52 +0200 Subject: [PATCH] Fixed immutable array reducer bug (#1591) * Fixed immutable array reducer bug * linting --- src/__tests__/FieldArray.spec.js | 478 ++++++++++++++++-- src/__tests__/actions.spec.js | 73 ++- src/__tests__/reducer.arrayPush.spec.js | 12 + src/reducer.js | 14 +- .../immutable/__tests__/splice.spec.js | 55 +- src/structure/immutable/expectations.js | 13 +- src/structure/immutable/index.js | 1 + src/structure/immutable/splice.js | 11 +- src/structure/plain/__tests__/splice.spec.js | 55 +- src/structure/plain/expectations.js | 9 + src/structure/plain/index.js | 1 + src/structure/plain/splice.js | 22 +- 12 files changed, 640 insertions(+), 104 deletions(-) diff --git a/src/__tests__/FieldArray.spec.js b/src/__tests__/FieldArray.spec.js index 2726bfc33..e3c9c763a 100644 --- a/src/__tests__/FieldArray.spec.js +++ b/src/__tests__/FieldArray.spec.js @@ -777,45 +777,461 @@ const describeFieldArray = (name, structure, combineReducers, expect) => { // field array NOT rerendered expect(renderFieldArray.calls.length).toBe(1) }) - }) - it('should work with Fields', () => { - const store = makeStore({ - testForm: { - values: { - foo: [ 'firstValue', 'secondValue' ] + it('should create a list in the store on push(undefined)', () => { + const store = makeStore({}) + const renderField = createSpy(props => ).andCallThrough() + const renderFieldArray = + createSpy(({ fields }) => (
+ {fields.map(field => )} + +
)).andCallThrough() + class Form extends Component { + render() { + return } } + const TestForm = reduxForm({ form: 'testForm' })(Form) + const dom = TestUtils.renderIntoDocument( + + + + ) + const button = TestUtils.findRenderedDOMComponentWithTag(dom, 'button') + + // only registered field array in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + registeredFields: [ + { name: 'dogs', type: 'FieldArray' } + ] + } + } + }) + + // length is 0 + expect(renderFieldArray).toHaveBeenCalled() + expect(renderFieldArray.calls.length).toBe(1) + expect(renderFieldArray.calls[ 0 ].arguments[ 0 ].fields.length).toBe(0) + + // add field + TestUtils.Simulate.click(button) + + // field array rerendered + expect(renderFieldArray.calls.length).toBe(2) + expect(renderFieldArray.calls[ 1 ].arguments[ 0 ].fields.length).toBe(1) + + // field rendered + expect(renderField).toHaveBeenCalled() + expect(renderField.calls.length).toBe(1) + expect(renderField.calls[ 0 ].arguments[ 0 ].input.name).toBe('dogs[0]') + expect(renderField.calls[ 0 ].arguments[ 0 ].input.value).toBe('') + + // field registered in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + values: { + dogs: [ undefined ] + }, + registeredFields: [ + { name: 'dogs', type: 'FieldArray' }, + { name: 'dogs[0]', type: 'Field' } + ] + } + } + }) + + // values list is a list + expect(getIn(store.getState(), 'form.testForm.values.dogs')).toBeAList() }) - const renderField = createSpy(field => ) - const renderFields = createSpy(({ foo }) =>
{foo.map(renderField)}
).andCallThrough() + it('should create a list in the store on push(value)', () => { + const store = makeStore({}) + const renderField = createSpy(props => ).andCallThrough() + const renderFieldArray = + createSpy(({ fields }) => (
+ {fields.map(field => )} + +
)).andCallThrough() + class Form extends Component { + render() { + return + } + } + const TestForm = reduxForm({ form: 'testForm' })(Form) + const dom = TestUtils.renderIntoDocument( + + + + ) + const button = TestUtils.findRenderedDOMComponentWithTag(dom, 'button') + + // only registered field array in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + registeredFields: [ + { name: 'dogs', type: 'FieldArray' } + ] + } + } + }) - const component = createSpy(({ fields }) =>
- -
).andCallThrough() + // length is 0 + expect(renderFieldArray).toHaveBeenCalled() + expect(renderFieldArray.calls.length).toBe(1) + expect(renderFieldArray.calls[ 0 ].arguments[ 0 ].fields.length).toBe(0) - class Form extends Component { - render() { - return (
- -
) + // add field + TestUtils.Simulate.click(button) + + // field array rerendered + expect(renderFieldArray.calls.length).toBe(2) + expect(renderFieldArray.calls[ 1 ].arguments[ 0 ].fields.length).toBe(1) + + // field rendered + expect(renderField).toHaveBeenCalled() + expect(renderField.calls.length).toBe(1) + expect(renderField.calls[ 0 ].arguments[ 0 ].input.name).toBe('dogs[0]') + expect(renderField.calls[ 0 ].arguments[ 0 ].input.value).toBe('Fido') + + // field registered in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + values: { + dogs: [ 'Fido' ] + }, + registeredFields: [ + { name: 'dogs', type: 'FieldArray' }, + { name: 'dogs[0]', type: 'Field' } + ] + } + } + }) + + // values list is a list + expect(getIn(store.getState(), 'form.testForm.values.dogs')).toBeAList() + }) + + it('should create a list in the store on unshift(undefined)', () => { + const store = makeStore({}) + const renderField = createSpy(props => ).andCallThrough() + const renderFieldArray = + createSpy(({ fields }) => (
+ {fields.map(field => )} + +
)).andCallThrough() + class Form extends Component { + render() { + return + } } - } - const TestForm = reduxForm({ form: 'testForm' })(Form) - TestUtils.renderIntoDocument( - - - - ) - expect(renderFields).toHaveBeenCalled() - expect(renderFields.calls.length).toBe(1) - expect(renderFields.calls[ 0 ].arguments[ 0 ].foo.length).toBe(2) - - expect(renderField).toHaveBeenCalled() - expect(renderField.calls.length).toBe(2) - expect(renderField.calls[ 0 ].arguments[ 0 ].input.value).toBe('firstValue') - expect(renderField.calls[ 1 ].arguments[ 0 ].input.value).toBe('secondValue') + const TestForm = reduxForm({ form: 'testForm' })(Form) + const dom = TestUtils.renderIntoDocument( + + + + ) + const button = TestUtils.findRenderedDOMComponentWithTag(dom, 'button') + + // only registered field array in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + registeredFields: [ + { name: 'dogs', type: 'FieldArray' } + ] + } + } + }) + + // length is 0 + expect(renderFieldArray).toHaveBeenCalled() + expect(renderFieldArray.calls.length).toBe(1) + expect(renderFieldArray.calls[ 0 ].arguments[ 0 ].fields.length).toBe(0) + + // add field + TestUtils.Simulate.click(button) + + // field array rerendered + expect(renderFieldArray.calls.length).toBe(2) + expect(renderFieldArray.calls[ 1 ].arguments[ 0 ].fields.length).toBe(1) + + // field rendered + expect(renderField).toHaveBeenCalled() + expect(renderField.calls.length).toBe(1) + expect(renderField.calls[ 0 ].arguments[ 0 ].input.name).toBe('dogs[0]') + expect(renderField.calls[ 0 ].arguments[ 0 ].input.value).toBe('') + + // field registered in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + values: { + dogs: [ undefined ] + }, + registeredFields: [ + { name: 'dogs', type: 'FieldArray' }, + { name: 'dogs[0]', type: 'Field' } + ] + } + } + }) + + // values list is a list + expect(getIn(store.getState(), 'form.testForm.values.dogs')).toBeAList() + }) + + it('should create a list in the store on unshift(value)', () => { + const store = makeStore({}) + const renderField = createSpy(props => ).andCallThrough() + const renderFieldArray = + createSpy(({ fields }) => (
+ {fields.map(field => )} + +
)).andCallThrough() + class Form extends Component { + render() { + return + } + } + const TestForm = reduxForm({ form: 'testForm' })(Form) + const dom = TestUtils.renderIntoDocument( + + + + ) + const button = TestUtils.findRenderedDOMComponentWithTag(dom, 'button') + + // only registered field array in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + registeredFields: [ + { name: 'dogs', type: 'FieldArray' } + ] + } + } + }) + + // length is 0 + expect(renderFieldArray).toHaveBeenCalled() + expect(renderFieldArray.calls.length).toBe(1) + expect(renderFieldArray.calls[ 0 ].arguments[ 0 ].fields.length).toBe(0) + + // add field + TestUtils.Simulate.click(button) + + // field array rerendered + expect(renderFieldArray.calls.length).toBe(2) + expect(renderFieldArray.calls[ 1 ].arguments[ 0 ].fields.length).toBe(1) + + // field rendered + expect(renderField).toHaveBeenCalled() + expect(renderField.calls.length).toBe(1) + expect(renderField.calls[ 0 ].arguments[ 0 ].input.name).toBe('dogs[0]') + expect(renderField.calls[ 0 ].arguments[ 0 ].input.value).toBe('Fido') + + // field registered in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + values: { + dogs: [ 'Fido' ] + }, + registeredFields: [ + { name: 'dogs', type: 'FieldArray' }, + { name: 'dogs[0]', type: 'Field' } + ] + } + } + }) + + // values list is a list + expect(getIn(store.getState(), 'form.testForm.values.dogs')).toBeAList() + }) + + + it('should create a list in the store on insert(undefined)', () => { + const store = makeStore({}) + const renderField = createSpy(props => ).andCallThrough() + const renderFieldArray = + createSpy(({ fields }) => (
+ {fields.map(field => )} + +
)).andCallThrough() + class Form extends Component { + render() { + return + } + } + const TestForm = reduxForm({ form: 'testForm' })(Form) + const dom = TestUtils.renderIntoDocument( + + + + ) + const button = TestUtils.findRenderedDOMComponentWithTag(dom, 'button') + + // only registered field array in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + registeredFields: [ + { name: 'dogs', type: 'FieldArray' } + ] + } + } + }) + + // length is 0 + expect(renderFieldArray).toHaveBeenCalled() + expect(renderFieldArray.calls.length).toBe(1) + expect(renderFieldArray.calls[ 0 ].arguments[ 0 ].fields.length).toBe(0) + + // add field + TestUtils.Simulate.click(button) + + // field array rerendered + expect(renderFieldArray.calls.length).toBe(2) + expect(renderFieldArray.calls[ 1 ].arguments[ 0 ].fields.length).toBe(1) + + // field rendered + expect(renderField).toHaveBeenCalled() + expect(renderField.calls.length).toBe(1) + expect(renderField.calls[ 0 ].arguments[ 0 ].input.name).toBe('dogs[0]') + expect(renderField.calls[ 0 ].arguments[ 0 ].input.value).toBe('') + + // field registered in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + values: { + dogs: [ undefined ] + }, + registeredFields: [ + { name: 'dogs', type: 'FieldArray' }, + { name: 'dogs[0]', type: 'Field' } + ] + } + } + }) + + // values list is a list + expect(getIn(store.getState(), 'form.testForm.values.dogs')).toBeAList() + }) + + it('should create a list in the store on insert(value)', () => { + const store = makeStore({}) + const renderField = createSpy(props => ).andCallThrough() + const renderFieldArray = + createSpy(({ fields }) => (
+ {fields.map(field => )} + +
)).andCallThrough() + class Form extends Component { + render() { + return + } + } + const TestForm = reduxForm({ form: 'testForm' })(Form) + const dom = TestUtils.renderIntoDocument( + + + + ) + const button = TestUtils.findRenderedDOMComponentWithTag(dom, 'button') + + // only registered field array in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + registeredFields: [ + { name: 'dogs', type: 'FieldArray' } + ] + } + } + }) + + // length is 0 + expect(renderFieldArray).toHaveBeenCalled() + expect(renderFieldArray.calls.length).toBe(1) + expect(renderFieldArray.calls[ 0 ].arguments[ 0 ].fields.length).toBe(0) + + // add field + TestUtils.Simulate.click(button) + + // field array rerendered + expect(renderFieldArray.calls.length).toBe(2) + expect(renderFieldArray.calls[ 1 ].arguments[ 0 ].fields.length).toBe(1) + + // field rendered + expect(renderField).toHaveBeenCalled() + expect(renderField.calls.length).toBe(1) + expect(renderField.calls[ 0 ].arguments[ 0 ].input.name).toBe('dogs[0]') + expect(renderField.calls[ 0 ].arguments[ 0 ].input.value).toBe('Fido') + + // field registered in store + expect(store.getState()).toEqualMap({ + form: { + testForm: { + values: { + dogs: [ 'Fido' ] + }, + registeredFields: [ + { name: 'dogs', type: 'FieldArray' }, + { name: 'dogs[0]', type: 'Field' } + ] + } + } + }) + + // values list is a list + expect(getIn(store.getState(), 'form.testForm.values.dogs')).toBeAList() + }) + + it('should work with Fields', () => { + const store = makeStore({ + testForm: { + values: { + foo: [ 'firstValue', 'secondValue' ] + } + } + }) + const renderField = createSpy(field => ) + + const renderFields = createSpy(({ foo }) => +
{foo.map(renderField)}
).andCallThrough() + + const component = createSpy(({ fields }) =>
+ +
).andCallThrough() + + class Form extends Component { + render() { + return (
+ +
) + } + } + const TestForm = reduxForm({ form: 'testForm' })(Form) + TestUtils.renderIntoDocument( + + + + ) + expect(renderFields).toHaveBeenCalled() + expect(renderFields.calls.length).toBe(1) + expect(renderFields.calls[ 0 ].arguments[ 0 ].foo.length).toBe(2) + + expect(renderField).toHaveBeenCalled() + expect(renderField.calls.length).toBe(2) + expect(renderField.calls[ 0 ].arguments[ 0 ].input.value).toBe('firstValue') + expect(renderField.calls[ 1 ].arguments[ 0 ].input.value).toBe('secondValue') + }) }) } diff --git a/src/__tests__/actions.spec.js b/src/__tests__/actions.spec.js index 1bb89d23b..39ed32dd8 100644 --- a/src/__tests__/actions.spec.js +++ b/src/__tests__/actions.spec.js @@ -1,16 +1,62 @@ import expect from 'expect' import expectPredicate from 'expect-predicate' import { - ARRAY_INSERT, ARRAY_MOVE, ARRAY_POP, ARRAY_PUSH, ARRAY_REMOVE, ARRAY_REMOVE_ALL, ARRAY_SHIFT, - ARRAY_SPLICE, ARRAY_SWAP, ARRAY_UNSHIFT, BLUR, CHANGE, DESTROY, FOCUS, INITIALIZE, - REGISTER_FIELD, RESET, SET_SUBMIT_FAILED, SET_SUBMIT_SUCCEEDED, START_ASYNC_VALIDATION, START_SUBMIT, - STOP_ASYNC_VALIDATION, STOP_SUBMIT, TOUCH, UNREGISTER_FIELD, UNTOUCH, UPDATE_SYNC_ERRORS + ARRAY_INSERT, + ARRAY_MOVE, + ARRAY_POP, + ARRAY_PUSH, + ARRAY_REMOVE, + ARRAY_REMOVE_ALL, + ARRAY_SHIFT, + ARRAY_SPLICE, + ARRAY_SWAP, + ARRAY_UNSHIFT, + BLUR, + CHANGE, + DESTROY, + FOCUS, + INITIALIZE, + REGISTER_FIELD, + RESET, + SET_SUBMIT_FAILED, + SET_SUBMIT_SUCCEEDED, + START_ASYNC_VALIDATION, + START_SUBMIT, + STOP_ASYNC_VALIDATION, + STOP_SUBMIT, + TOUCH, + UNREGISTER_FIELD, + UNTOUCH, + UPDATE_SYNC_ERRORS } from '../actionTypes' import { - arrayInsert, arrayMove, arrayPop, arrayPush, arrayRemove, arrayRemoveAll, arrayShift, - arraySplice, arraySwap, arrayUnshift, blur, change, destroy, focus, initialize, registerField, - reset, setSubmitFailed, setSubmitSucceeded, startAsyncValidation, startSubmit, stopAsyncValidation, - stopSubmit, touch, unregisterField, untouch, updateSyncErrors + arrayInsert, + arrayMove, + arrayPop, + arrayPush, + arrayRemove, + arrayRemoveAll, + arrayShift, + arraySplice, + arraySwap, + arrayUnshift, + blur, + change, + destroy, + focus, + initialize, + registerField, + reset, + setSubmitFailed, + setSubmitSucceeded, + startAsyncValidation, + startSubmit, + stopAsyncValidation, + stopSubmit, + touch, + unregisterField, + untouch, + updateSyncErrors } from '../actions' import { isFSA } from 'flux-standard-action' expect.extend(expectPredicate) @@ -68,6 +114,17 @@ describe('actions', () => { payload: 'foo' }) .toPass(isFSA) + + expect(arrayPush('myForm', 'myField')) + .toEqual({ + type: ARRAY_PUSH, + meta: { + form: 'myForm', + field: 'myField' + }, + payload: undefined + }) + .toPass(isFSA) }) it('should create array remove action', () => { diff --git a/src/__tests__/reducer.arrayPush.spec.js b/src/__tests__/reducer.arrayPush.spec.js index 80cbbb540..26c1783d8 100644 --- a/src/__tests__/reducer.arrayPush.spec.js +++ b/src/__tests__/reducer.arrayPush.spec.js @@ -13,6 +13,18 @@ const describeArrayPush = (reducer, expect, { fromJS }) => () => { }) }) + it('should work pushing undefined to empty state', () => { + const state = reducer(undefined, arrayPush('foo', 'myField')) + expect(state) + .toEqualMap({ + foo: { + values: { + myField: [ undefined ] + } + } + }) + }) + it('should work with existing empty array', () => { const state = reducer(fromJS({ foo: { diff --git a/src/reducer.js b/src/reducer.js index 37fffa361..af5ddee11 100644 --- a/src/reducer.js +++ b/src/reducer.js @@ -31,7 +31,17 @@ import 'array-findindex-polyfill' import createDeleteInWithCleanUp from './deleteInWithCleanUp' const createReducer = structure => { - const { splice, empty, getIn, setIn, deleteIn, fromJS, size, some, deepEqual } = structure + const { + deepEqual, + empty, + getIn, + setIn, + deleteIn, + fromJS, + size, + some, + splice + } = structure const deleteInWithCleanUp = createDeleteInWithCleanUp(structure) const doSplice = (state, key, field, index, removeNum, value, force) => { const existing = getIn(state, `${key}.${field}`) @@ -75,7 +85,7 @@ const createReducer = structure => { const length = array ? size(array) : 0 return length ? arraySplice(state, field, length - 1, 1) : state }, - [ARRAY_PUSH](state, { meta: { field }, payload = empty }) { + [ARRAY_PUSH](state, { meta: { field }, payload }) { const array = getIn(state, `values.${field}`) const length = array ? size(array) : 0 return arraySplice(state, field, length, 0, payload) diff --git a/src/structure/immutable/__tests__/splice.spec.js b/src/structure/immutable/__tests__/splice.spec.js index 4817139b7..db2969623 100644 --- a/src/structure/immutable/__tests__/splice.spec.js +++ b/src/structure/immutable/__tests__/splice.spec.js @@ -3,35 +3,40 @@ import { fromJS, List } from 'immutable' import splice from '../splice' describe('structure.immutable.splice', () => { - it('should insert even when initial array is undefined', () => { - expect(splice(undefined, 2, 0, 'foo')) // really goes to index 0 - .toBeA(List) - .toEqual(fromJS([ , , 'foo' ])) // eslint-disable-line no-sparse-arrays - }) + const testInsertWithValue = value => { + it('should insert even when initial array is undefined', () => { + expect(splice(undefined, 2, 0, value)) // really goes to index 0 + .toBeA(List) + .toEqual(fromJS([ , , value ])) // eslint-disable-line no-sparse-arrays + }) - it('should insert at start', () => { - expect(splice(fromJS([ 'b', 'c', 'd' ]), 0, 0, 'a')) - .toBeA(List) - .toEqual(fromJS([ 'a', 'b', 'c', 'd' ])) - }) + it('should insert at start', () => { + expect(splice(fromJS([ 'b', 'c', 'd' ]), 0, 0, value)) + .toBeA(List) + .toEqual(fromJS([ value, 'b', 'c', 'd' ])) + }) - it('should insert at end', () => { - expect(splice(fromJS([ 'a', 'b', 'c' ]), 3, 0, 'd')) - .toBeA(List) - .toEqual(fromJS([ 'a', 'b', 'c', 'd' ])) - }) + it('should insert at end', () => { + expect(splice(fromJS([ 'a', 'b', 'c' ]), 3, 0, value)) + .toBeA(List) + .toEqual(fromJS([ 'a', 'b', 'c', value ])) + }) - it('should insert in middle', () => { - expect(splice(fromJS([ 'a', 'b', 'd' ]), 2, 0, 'c')) - .toBeA(List) - .toEqual(fromJS([ 'a', 'b', 'c', 'd' ])) - }) + it('should insert in middle', () => { + expect(splice(fromJS([ 'a', 'b', 'd' ]), 2, 0, value)) + .toBeA(List) + .toEqual(fromJS([ 'a', 'b', value, 'd' ])) + }) - it('should insert in out of range', () => { - expect(splice(fromJS([ 'a', 'b', 'c' ]), 5, 0, 'f')) - .toBeA(List) - .toEqual(fromJS([ 'a', 'b', 'c', , , 'f' ])) // eslint-disable-line no-sparse-arrays - }) + it('should insert in out of range', () => { + expect(splice(fromJS([ 'a', 'b', 'c' ]), 5, 0, value)) + .toBeA(List) + .toEqual(fromJS([ 'a', 'b', 'c', , , value ])) // eslint-disable-line no-sparse-arrays + }) + } + + testInsertWithValue('value') + testInsertWithValue(undefined) it('should return empty array when removing and initial array is undefined', () => { expect(splice(undefined, 2, 1)) diff --git a/src/structure/immutable/expectations.js b/src/structure/immutable/expectations.js index 29fe5de99..4aacb4be4 100644 --- a/src/structure/immutable/expectations.js +++ b/src/structure/immutable/expectations.js @@ -1,6 +1,6 @@ import expect from 'expect' import deepEqual from 'deep-equal' -import { Map, Iterable, fromJS } from 'immutable' +import { Map, List, Iterable, fromJS } from 'immutable' const deepEqualValues = (a, b) => { if (Iterable.isIterable(a)) { @@ -15,7 +15,16 @@ const api = { toBeAMap() { expect.assert( Map.isMap(this.actual), - 'expected %s to be an immutable map', + 'expected %s to be an immutable Map', + this.actual + ) + return this + }, + + toBeAList() { + expect.assert( + List.isList(this.actual), + 'expected %s to be an immutable List', this.actual ) return this diff --git a/src/structure/immutable/index.js b/src/structure/immutable/index.js index 80893d086..0bf589c08 100644 --- a/src/structure/immutable/index.js +++ b/src/structure/immutable/index.js @@ -6,6 +6,7 @@ import plainGetIn from '../plain/getIn' const structure = { empty: Map(), + emptyList: List(), getIn: (state, field) => Map.isMap(state) || List.isList(state) ? state.getIn(toPath(field)) : plainGetIn(state, field), setIn: (state, field, value) => state.setIn(toPath(field), value), diff --git a/src/structure/immutable/splice.js b/src/structure/immutable/splice.js index ecb3642ec..50ee0dbd3 100644 --- a/src/structure/immutable/splice.js +++ b/src/structure/immutable/splice.js @@ -2,14 +2,19 @@ import { List } from 'immutable' export default (list = List.isList(list) || List(), index, removeNum, value) => { if (index < list.count()) { + if (value === undefined && !removeNum) { // inserting undefined + // first insert null and then re-set it to undefined + return list.splice(index, 0, null).set(index, undefined) + } if (value != null) { return list.splice(index, removeNum, value) // removing and adding } else { return list.splice(index, removeNum) // removing } } - if (value != null) { - return list.set(index, value) + if (removeNum) { // trying to remove non-existant item: return original array + return list } - return list + // trying to add outside of range: just set value + return list.set(index, value) } diff --git a/src/structure/plain/__tests__/splice.spec.js b/src/structure/plain/__tests__/splice.spec.js index 149ef5b55..b0811aee8 100644 --- a/src/structure/plain/__tests__/splice.spec.js +++ b/src/structure/plain/__tests__/splice.spec.js @@ -2,35 +2,40 @@ import expect from 'expect' import splice from '../splice' describe('structure.plain.splice', () => { - it('should insert even when initial array is undefined', () => { - expect(splice(undefined, 2, 0, 'foo')) // really goes to index 0 - .toBeA('array') - .toEqual([ , , 'foo' ]) // eslint-disable-line no-sparse-arrays - }) + const testInsertWithValue = value => { + it('should insert even when initial array is undefined', () => { + expect(splice(undefined, 2, 0, value)) // really goes to index 0 + .toBeA('array') + .toEqual([ , , value ]) // eslint-disable-line no-sparse-arrays + }) - it('should insert at start', () => { - expect(splice([ 'b', 'c', 'd' ], 0, 0, 'a')) - .toBeA('array') - .toEqual([ 'a', 'b', 'c', 'd' ]) - }) + it(`should insert ${value} at start`, () => { + expect(splice([ 'b', 'c', 'd' ], 0, 0, value)) + .toBeA('array') + .toEqual([ value, 'b', 'c', 'd' ]) + }) - it('should insert at end', () => { - expect(splice([ 'a', 'b', 'c' ], 3, 0, 'd')) - .toBeA('array') - .toEqual([ 'a', 'b', 'c', 'd' ]) - }) + it(`should insert ${value} at end`, () => { + expect(splice([ 'a', 'b', 'c' ], 3, 0, value)) + .toBeA('array') + .toEqual([ 'a', 'b', 'c', value ]) + }) - it('should insert in middle', () => { - expect(splice([ 'a', 'b', 'd' ], 2, 0, 'c')) - .toBeA('array') - .toEqual([ 'a', 'b', 'c', 'd' ]) - }) + it(`should insert ${value} in middle`, () => { + expect(splice([ 'a', 'b', 'd' ], 2, 0, value)) + .toBeA('array') + .toEqual([ 'a', 'b', value, 'd' ]) + }) - it('should insert in out of range', () => { - expect(splice([ 'a', 'b', 'c' ], 5, 0, 'f')) - .toBeA('array') - .toEqual([ 'a', 'b', 'c', , , 'f' ]) // eslint-disable-line no-sparse-arrays - }) + it(`should insert ${value} when index is out of range`, () => { + expect(splice([ 'a', 'b', 'c' ], 5, 0, value)) + .toBeA('array') + .toEqual([ 'a', 'b', 'c', , , value ]) // eslint-disable-line no-sparse-arrays + }) + } + + testInsertWithValue('value') + testInsertWithValue(undefined) it('should return empty array when removing and initial array is undefined', () => { expect(splice(undefined, 2, 1)) diff --git a/src/structure/plain/expectations.js b/src/structure/plain/expectations.js index 72173db4e..f477d6522 100644 --- a/src/structure/plain/expectations.js +++ b/src/structure/plain/expectations.js @@ -11,6 +11,15 @@ const expectations = { return this }, + toBeAList() { + expect.assert( + Array.isArray(this.actual), + 'expected %s to be an array', + this.actual + ) + return this + }, + toBeSize(size) { expect.assert( this.actual && Object.keys(this.actual).length === size, diff --git a/src/structure/plain/index.js b/src/structure/plain/index.js index ccaadd543..aa5ce8101 100644 --- a/src/structure/plain/index.js +++ b/src/structure/plain/index.js @@ -7,6 +7,7 @@ import { some } from 'lodash' const structure = { empty: {}, + emptyList: [], getIn, setIn, deepEqual, diff --git a/src/structure/plain/splice.js b/src/structure/plain/splice.js index a2d07dc10..2e3419f4d 100644 --- a/src/structure/plain/splice.js +++ b/src/structure/plain/splice.js @@ -1,21 +1,27 @@ const splice = (array = [], index, removeNum, value) => { if (index < array.length) { - if (value != null) { + if (value === undefined && !removeNum) { // inserting undefined const copy = [ ...array ] - copy.splice(index, removeNum, value) // removing and adding + copy.splice(index, 0, null) + copy[ index ] = undefined return copy - } else { + } + if (value != null) { const copy = [ ...array ] - copy.splice(index, removeNum) // removing + copy.splice(index, removeNum, value) // removing and adding return copy } - } - if (value != null) { const copy = [ ...array ] - copy[index] = value + copy.splice(index, removeNum) // removing return copy } - return array + if (removeNum) { // trying to remove non-existant item: return original array + return array + } + // trying to add outside of range: just set value + const copy = [ ...array ] + copy[ index ] = value + return copy } export default splice