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.

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

Supporting a new set methods #3853

Merged
merged 25 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6572115
3850 - if you're using the corepack (https://nodejs.org/api/corepack.…
inoyakaigor Apr 2, 2024
eabcdde
3850 - first implementation of new Set methods
inoyakaigor Apr 2, 2024
9763039
3850 - call a method on the argument
inoyakaigor Apr 15, 2024
7131add
3850 - Add code to few another Set methods; align typings to TS PR#57230
inoyakaigor Apr 18, 2024
6b93c8a
3850 - review fixes: move reportObserved() into conditions branch; ch…
inoyakaigor Apr 18, 2024
4e438bb
3850 - add rest of the Set methods
inoyakaigor Apr 26, 2024
f5d6863
3850 - align functions parameter type declarations with its expected …
inoyakaigor May 2, 2024
46df92b
3850 - update TS to version with new Set methods; remove copied types…
inoyakaigor May 17, 2024
4a258c7
3850 - fix type for the «difference» function
inoyakaigor May 17, 2024
cf22937
3850 - add ESNext to tsconfig lib to fix tests
inoyakaigor May 17, 2024
a109642
3850 - update TS to Beta
inoyakaigor Jun 6, 2024
4767248
3850 - fix build failing (finally!)
inoyakaigor Jun 6, 2024
538ac5a
3850 - add tests for Set methods
inoyakaigor Jun 11, 2024
30fa622
3850 - change test for Set methods; change source code for methods wh…
inoyakaigor Jun 16, 2024
615b9e7
3850 - bump Nodejs version to 22 because it is support new Set method…
inoyakaigor Jun 16, 2024
68887de
Merge branch 'master' into 3850-new-set-methods
inoyakaigor Jun 16, 2024
7b7317b
3850 – tests for set-like objects
inoyakaigor Jun 19, 2024
74a307a
3850 – tests for check reactivity
inoyakaigor Jun 19, 2024
8ca04e6
3850 – update deps to TS 5.5; remove a packageManager field from pack…
inoyakaigor Jun 25, 2024
5f06ab2
3850 – cancel installation.md changes
inoyakaigor Jun 25, 2024
00d4cc9
3850 – esnext already contains es6
inoyakaigor Jun 30, 2024
81c2329
3850 – change types for new methods and checks for otherSet is has Se…
inoyakaigor Jul 1, 2024
113558b
3850 – review fixes: clarify the types
inoyakaigor Jul 1, 2024
4acd882
3850 – review fixes: removed unnecessary code
inoyakaigor Jul 2, 2024
d68dd2a
Merge branch 'main' into 3850-new-set-methods
urugator Jul 2, 2024
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
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,12 @@
"tape": "^5.0.1",
"ts-jest": "^29.0.5",
"tsdx": "^0.14.1",
"typescript": "^5.0.2"
"typescript": "^5.5.0-dev.20240415"
inoyakaigor marked this conversation as resolved.
Show resolved Hide resolved
},
"husky": {
"hooks": {
"pre-commit": "pretty-quick --staged"
}
}
},
"packageManager": "yarn@1.22.19+sha256.732620bac8b1690d507274f025f3c6cfdc3627a84d9642e38a07452cc00e0f2e"
inoyakaigor marked this conversation as resolved.
Show resolved Hide resolved
}
17 changes: 17 additions & 0 deletions packages/mobx/src/global.d.ts
Original file line number Diff line number Diff line change
@@ -1 +1,18 @@
declare const __DEV__: boolean

// This code is a copy from TS PR#57230
// Should be removed after it merged
interface ReadonlySetLike<T> {
/**
* Despite its name, returns an iterable of the values in the set-like.
*/
keys(): Iterator<T>
/**
* @returns a boolean indicating whether an element with the specified value exists in the set-like or not.
*/
has(value: T): boolean
/**
* @returns the number of (unique) elements in the set-like.
*/
readonly size: number
}
56 changes: 56 additions & 0 deletions packages/mobx/src/types/observableset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,62 @@ export class ObservableSet<T = any> implements Set<T>, IInterceptable<ISetWillCh
} as any)
}

intersection<U>(otherSet: ReadonlySetLike<U>): Set<T & U> {
this.atom_.reportObserved()
inoyakaigor marked this conversation as resolved.
Show resolved Hide resolved

if (typeof otherSet.intersection !== "function") {
inoyakaigor marked this conversation as resolved.
Show resolved Hide resolved
return otherSet.intersection(this.data_)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't correct, it won't work with custom enhancer. Was there any problem with the code I've proposed originally? That is:

if (isES6Set(otherSet)) {
   return otherSet.intersection(this)
} else {
  // ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've checked twice where you mentioned about deleting reportObserved() but didn't find. Maybe you're assumed this but I didn't get it. So anyway removing the reportObserved() throwing errors:

image

Copy link
Collaborator

@urugator urugator Jul 2, 2024

Choose a reason for hiding this comment

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

where you mentioned about deleting reportObserved() but didn't find.

I didn't mention deleting reportObserved anywhere, because I didn't propose adding it in the first place:
#3853 (comment)

So anyway removing the reportObserved() throwing errors:

Did you just removed reportObserved or did you also replaced this._data with this as suggested?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I've tought you're mentioned it in your latest messages. Totally forgot about this one! In this case everything is okay

} else {
const dehancedSet = new Set(this)
return dehancedSet.intersection(otherSet)
}
}

union<U>(otherSet: ReadonlySetLike<U>): Set<T | U> {
this.atom_.reportObserved()

if (typeof otherSet.union !== "function") {
return otherSet.union(this.data_)
} else {
const dehancedSet = new Set(this)
return dehancedSet.union(otherSet)
}
}

difference<U>(otherSet: ReadonlySetLike<U>): Set<T> {
this.atom_.reportObserved()

if (typeof otherSet.difference !== "function") {
return otherSet.difference(this.data_)
} else {
const dehancedSet = new Set(this)
return dehancedSet.difference(otherSet)
}
}

symmetricDifference<U>(otherSet: ReadonlySetLike<U>): Set<T | U> {
this.atom_.reportObserved()

if (typeof otherSet.symmetricDifference !== "function") {
return otherSet.symmetricDifference(this.data_)
} else {
const dehancedSet = new Set(this)
return dehancedSet.symmetricDifference(otherSet)
}
}

isSubsetOf(): IterableIterator<T> {
// TODO: implement
}

isSupersetOf(): IterableIterator<T> {
// TODO: implement
}

isDisjointFrom(): IterableIterator<T> {
// TODO: implement
}

replace(other: ObservableSet<T> | IObservableSetInitialValues<T>): ObservableSet<T> {
if (isObservableSet(other)) {
other = new Set(other)
Expand Down
Loading
Loading