Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix use of notSetValue in updateIn #1658

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix use of notSetValue in updateIn #1658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d47469
Fix use of notSetValue in updateIn
migueloller Nov 30, 2018
982cade
Remove use of internal NOT_SET from updateIn
migueloller Nov 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions __tests__/updateIn.ts
View file
Open in desktop
Expand Up @@ -31,6 +31,14 @@ describe('updateIn', () => {
});
});

it('remove in raw JS', () => {
const m = { a: { b: { c: 10 } } };
const NOT_SET = { NOT_SET: true };
expect(updateIn(m, ['a', 'b', 'c'], NOT_SET, () => NOT_SET)).toEqual({
a: { b: {} },
});
});

it('deep edit throws without list or array-like', () => {
// need to cast these as TypeScript first prevents us from such clownery.
expect(() => Map().updateIn(undefined as any, x => x)).toThrow(
Expand Down
3 changes: 1 addition & 2 deletions src/functional/removeIn.js
View file
Open in desktop
Expand Up @@ -6,8 +6,7 @@
*/

import { updateIn } from './updateIn';
import { NOT_SET } from '../TrieUtils';

export function removeIn(collection, keyPath) {
return updateIn(collection, keyPath, () => NOT_SET);
return updateIn(collection, keyPath, () => undefined);
}
19 changes: 8 additions & 11 deletions src/functional/updateIn.js
View file
Open in desktop
Expand Up @@ -9,7 +9,6 @@ import { isImmutable } from '../predicates/isImmutable';
import coerceKeyPath from '../utils/coerceKeyPath';
import isDataStructure from '../utils/isDataStructure';
import quoteString from '../utils/quoteString';
import { NOT_SET } from '../TrieUtils';
import { emptyMap } from '../Map';
import { get } from './get';
import { remove } from './remove';
Expand All @@ -28,7 +27,7 @@ export function updateIn(collection, keyPath, notSetValue, updater) {
notSetValue,
updater
);
return updatedValue === NOT_SET ? notSetValue : updatedValue;
return updatedValue;
}

function updateInDeeply(
Expand All @@ -39,12 +38,8 @@ function updateInDeeply(
notSetValue,
updater
) {
const wasNotSet = existing === NOT_SET;
if (i === keyPath.length) {
const existingValue = wasNotSet ? notSetValue : existing;
const newValue = updater(existingValue);
return newValue === existingValue ? existing : newValue;
}
const wasNotSet = existing === notSetValue;
if (i === keyPath.length) return updater(wasNotSet ? notSetValue : existing);
if (!wasNotSet && !isDataStructure(existing)) {
throw new TypeError(
'Cannot update within non-data-structure value in path [' +
Expand All @@ -54,9 +49,11 @@ function updateInDeeply(
);
}
const key = keyPath[i];
const nextExisting = wasNotSet ? NOT_SET : get(existing, key, NOT_SET);
const nextExisting = wasNotSet
? notSetValue
: get(existing, key, notSetValue);
const nextUpdated = updateInDeeply(
nextExisting === NOT_SET ? inImmutable : isImmutable(nextExisting),
nextExisting === notSetValue ? inImmutable : isImmutable(nextExisting),
nextExisting,
keyPath,
i + 1,
Expand All @@ -65,7 +62,7 @@ function updateInDeeply(
);
return nextUpdated === nextExisting
? existing
: nextUpdated === NOT_SET
: nextUpdated === notSetValue
? remove(existing, key)
: set(
wasNotSet ? (inImmutable ? emptyMap() : {}) : existing,
Expand Down