Skip to content

Add Dict.has & improve Dict helpers performance #7316

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.

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#### :rocket: New Feature

- Add `Dict.has` and double `Dict.forEachWithKey`/`Dict.mapValues` performance. https://github.com/rescript-lang/rescript/pull/7316
- Add popover attributes to JsxDOM.domProps. https://github.com/rescript-lang/rescript/pull/7317

#### :house: Internal
Expand Down
8 changes: 6 additions & 2 deletions lib/es6/Stdlib_Dict.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,25 @@ function forEach(dict, f) {
}

function forEachWithKey(dict, f) {
Object.entries(dict).forEach(param => f(param[1], param[0]));
Object.keys(dict).forEach(key => f(dict[key], key));
}

function mapValues(dict, f) {
let target = {};
forEachWithKey(dict, (value, key) => {
Object.keys(dict).forEach(key => {
let value = dict[key];
Comment on lines +18 to +19
Copy link
Member Author

target[key] = f(value);
});
return target;
}

let has = ((dict, key) => key in dict);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we want to use hasOwnProperty there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. There are not many prototype fields on dict object, but still this is invalid with in:

console.log("__proto__" in {}) // true

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


export {
$$delete$1 as $$delete,
forEach,
forEachWithKey,
mapValues,
has,
}
/* No side effect */
8 changes: 6 additions & 2 deletions lib/js/Stdlib_Dict.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,23 @@ function forEach(dict, f) {
}

function forEachWithKey(dict, f) {
Object.entries(dict).forEach(param => f(param[1], param[0]));
Object.keys(dict).forEach(key => f(dict[key], key));
}

function mapValues(dict, f) {
let target = {};
forEachWithKey(dict, (value, key) => {
Object.keys(dict).forEach(key => {
let value = dict[key];
target[key] = f(value);
});
return target;
}

let has = ((dict, key) => key in dict);

exports.$$delete = $$delete$1;
exports.forEach = forEach;
exports.forEachWithKey = forEachWithKey;
exports.mapValues = mapValues;
exports.has = has;
/* No side effect */
5 changes: 4 additions & 1 deletion runtime/Stdlib_Dict.res
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ let forEach = (dict, f) => {
dict->valuesToArray->Stdlib_Array.forEach(value => f(value))
}

@inline
let forEachWithKey = (dict, f) => {
dict->toArray->Stdlib_Array.forEach(((key, value)) => f(value, key))
dict->keysToArray->Stdlib_Array.forEach(key => f(dict->getUnsafe(key), key))
}

let mapValues = (dict, f) => {
Expand All @@ -39,3 +40,5 @@ let mapValues = (dict, f) => {
})
target
}

let has: (dict<'a>, string) => bool = %raw(`(dict, key) => key in dict`)
29 changes: 22 additions & 7 deletions runtime/Stdlib_Dict.resi
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Use `Dict.getUnsafe` only when you are sure the key exists (i.e. when iterating

## Examples
```rescript
let dict = Dict.fromArray([("key1", "value1"), ("key2", "value2")])
let dict = dict{"key1": "value1", "key2": "value2"}
let value = dict->Dict.getUnsafe("key1")
Console.log(value) // value1
```
Expand All @@ -30,7 +30,7 @@ Returns the value at the provided key, if it exists. Returns an option.

## Examples
```rescript
let dict = Dict.fromArray([("someKey", "someValue")])
let dict = dict{"someKey": "someValue"}

switch dict->Dict.get("someKey") {
| None => Console.log("Nope, didn't have the key.")
Expand Down Expand Up @@ -59,7 +59,7 @@ external set: (dict<'a>, string, 'a) => unit = ""

## Examples
```rescript
let dict = Dict.fromArray([("someKey", "someValue")])
let dict = dict{"someKey": "someValue"}

dict->Dict.delete("someKey")
```
Expand Down Expand Up @@ -189,7 +189,7 @@ external assign: (dict<'a>, dict<'a>) => dict<'a> = "Object.assign"

## Examples
```rescript
let dict = Dict.fromArray([("key1", "value1"), ("key2", "value2")])
let dict = dict{"key1": "value1", "key2": "value2"}
let dict2 = dict->Dict.copy

// Both log `["key1", "key2"]` here.
Expand All @@ -206,7 +206,7 @@ external copy: (@as(json`{}`) _, dict<'a>) => dict<'a> = "Object.assign"

## Examples
```rescript
let dict = Dict.fromArray([("key1", "value1"), ("key2", "value2")])
let dict = dict{"key1": "value1", "key2": "value2"}

dict->Dict.forEach(value => {
Console.log(value)
Expand All @@ -220,7 +220,7 @@ let forEach: (dict<'a>, 'a => unit) => unit

## Examples
```rescript
let dict = Dict.fromArray([("key1", "value1"), ("key2", "value2")])
let dict = dict{"key1": "value1", "key2": "value2"}

dict->Dict.forEachWithKey((value, key) => {
Console.log2(value, key)
Expand All @@ -235,10 +235,25 @@ let forEachWithKey: (dict<'a>, ('a, string) => unit) => unit
## Examples

```rescript
let dict = Dict.fromArray([("key1", 1), ("key2", 2)])
let dict = dict{"key1": 1, "key2": 2}

dict->Dict.mapValues(v => v + 10)->Dict.toArray // [("key1", 11), ("key2", 12)]
dict->Dict.mapValues(v => Int.toString(v))->Dict.toArray // [("key1", "1"), ("key2", "2")]
```
*/
let mapValues: (dict<'a>, 'a => 'b) => dict<'b>

/**
`has(dictionary, "key")` returns true if the "key" is present in the dictionary.

## Examples

```rescript
let dict = dict{"key1": Some(1), "key2": None}

dict->Dict.has("key1") // true
dict->Dict.has("key2") // true
dict->Dict.has("key3") // false
```
*/
let has: (dict<'a>, string) => bool
49 changes: 48 additions & 1 deletion tests/tests/src/DictTests.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Generated by ReScript, PLEASE EDIT WITH CARE

import * as Stdlib_Dict from "rescript/lib/es6/Stdlib_Dict.js";

let someString = "hello";

Expand Down Expand Up @@ -41,6 +42,51 @@ let PatternMatching = {
constrainedAsDict: constrainedAsDict
};

let dict = {
key1: 1,
key2: undefined
};

if (Stdlib_Dict.has(dict, "key1") !== true) {
throw {
RE_EXN_ID: "Assert_failure",
_1: [
"DictTests.res",
43,
2
],
Error: new Error()
};
}

if (Stdlib_Dict.has(dict, "key2") !== true) {
throw {
RE_EXN_ID: "Assert_failure",
_1: [
"DictTests.res",
44,
2
],
Error: new Error()
};
}

if (Stdlib_Dict.has(dict, "key3") !== false) {
throw {
RE_EXN_ID: "Assert_failure",
_1: [
"DictTests.res",
45,
2
],
Error: new Error()
};
}

let DictHas = {
dict: dict
};

let three = 3;

export {
Expand All @@ -49,5 +95,6 @@ export {
three,
intDict,
PatternMatching,
DictHas,
}
/* No side effect */
/* Not a pure module */
11 changes: 11 additions & 0 deletions tests/tests/src/DictTests.res
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,14 @@ module PatternMatching = {
| _ => Js.log("not one")
}
}

module DictHas = {
let dict = dict{
"key1": Some(1),
"key2": None,
}

assert(dict->Dict.has("key1") === true)
assert(dict->Dict.has("key2") === true)
assert(dict->Dict.has("key3") === false)
}