Skip to content

🥅 Stricter types in apply()#4

Merged
alecgibson merged 1 commit into
masterfrom
type-checking
Jul 8, 2021
Merged

🥅 Stricter types in apply()#4
alecgibson merged 1 commit into
masterfrom
type-checking

Conversation

@alecgibson
Copy link
Copy Markdown
Collaborator

Fixes: ottypes#37

Background

There are some corner cases that arise in the json0 library because -
given an object obj, and an array arr -these statements are both
true in JavaScript:

obj['123'] === obj[123]
arr['123'] === arr[123]

The fact that these statements are true can lead to some unexpected
silent transform() failures:

const op1 = [{p: ['a', '1', 0], si: 'hi'}]
const op2 = [{p: ['a', 1], lm: 0}]

json0.transform(op1, op2, 'left')

Actual result: [{p: ["a", 2, 0], si: "hi"}]
Expected result: [{p: ["a", 0, 0], si: "hi"}]

Solution

In order to prevent this, it's been decided that arrays should always
be indexed by number, and objects should always be indexed by
string.

This change enforces stricter type checks when calling apply(), and
now throws in the following cases:

  • When a number is used to key an object property:
    type.apply({'1': 'a'}, [{p:[1], od: 'a'}])
  • When a string is used to key an array property:
    type.apply(['a'], [{p:['0'], ld: 'a'}])
  • When adding a string to a number:
    type.apply(1, [{p:[], na: 'a}])
  • When adding a number to a string:
    type.apply('a', [{p:[], na: 1}])
  • When applying a string operation to a non-string:
    type.apply(1, [{p: [0], si: 'a'}])

Fixes: ottypes#37

# Background

There are some corner cases that arise in the `json0` library because -
given an object `obj`, and an array `arr` -these statements are both
`true` in JavaScript:

```js
obj['123'] === obj[123]
arr['123'] === arr[123]
```

The fact that these statements are true can lead to some unexpected
silent `transform()` failures:

```js
const op1 = [{p: ['a', '1', 0], si: 'hi'}]
const op2 = [{p: ['a', 1], lm: 0}]

json0.transform(op1, op2, 'left')
```

Actual result: `[{p: ["a", 2, 0], si: "hi"}]`
Expected result: `[{p: ["a", 0, 0], si: "hi"}]`

# Solution

In order to prevent this, it's been decided that arrays should *always*
be indexed by `number`, and objects should *always* be indexed by
`string`.

This change enforces stricter type checks when calling `apply()`, and
now throws in the following cases:

 - When a `number` is used to key an object property:
   `type.apply({'1': 'a'}, [{p:[1], od: 'a'}])`
 - When a `string` is used to key an array property:
   `type.apply(['a'], [{p:['0'], ld: 'a'}])`
 - When adding a `string` to a `number`:
   `type.apply(1, [{p:[], na: 'a}])`
 - When adding a `number` to a `string`:
   `type.apply('a', [{p:[], na: 1}])`
 - When applying a string operation to a non-string:
   `type.apply(1, [{p: [0], si: 'a'}])`
@alecgibson alecgibson requested a review from fyvfyv July 7, 2021 10:26
@alecgibson alecgibson merged commit 53fe9eb into master Jul 8, 2021
@alecgibson alecgibson deleted the type-checking branch July 8, 2021 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Transforming ops with different use of integer/string indexes doesn't work

2 participants