Skip to content

Commit be74b5f

Browse files
wangyiz4262mweststrate
authored andcommitted
Refactor toJS (mobxjs#1699)
* The object will be recursively converted when calling toJS on it fix tests Fix some language inaccuracy * Retain object reference if plain when being converted Fix tests * Added option recurseEverything to toJS * Address code review feedback
1 parent ff78f69 commit be74b5f

2 files changed

Lines changed: 95 additions & 28 deletions

File tree

src/api/tojs.ts

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
11
import {
2+
keys,
23
isObservable,
34
isObservableArray,
4-
isObservableMap,
5-
isObservableObject,
65
isObservableValue,
7-
keys
6+
isObservableMap
87
} from "../internal"
98

109
export type ToJSOptions = {
1110
detectCycles?: boolean
1211
exportMapsAsObjects?: boolean
12+
recurseEverything?: boolean
1313
}
1414

1515
const defaultOptions: ToJSOptions = {
1616
detectCycles: true,
17-
exportMapsAsObjects: true
17+
exportMapsAsObjects: true,
18+
recurseEverything: false
1819
}
1920

2021
function cache<K, V>(map: Map<any, any>, key: K, value: V, options: ToJSOptions): V {
@@ -23,37 +24,35 @@ function cache<K, V>(map: Map<any, any>, key: K, value: V, options: ToJSOptions)
2324
}
2425

2526
function toJSHelper(source, options: ToJSOptions, __alreadySeen: Map<any, any>) {
26-
if (!isObservable(source)) return source
27+
if (!options.recurseEverything && !isObservable(source)) return source
28+
29+
if (typeof source !== "object") return source
30+
31+
// Directly return the Date object itself if contained in the observable
32+
if (source instanceof Date) return source
33+
34+
if (isObservableValue(source)) return toJSHelper(source.get(), options!, __alreadySeen)
35+
36+
// make sure we track the keys of the object
37+
if (isObservable(source)) {
38+
keys(source)
39+
}
2740

2841
const detectCycles = options.detectCycles === true
2942

30-
if (
31-
detectCycles &&
32-
source !== null &&
33-
typeof source === "object" &&
34-
__alreadySeen.has(source)
35-
) {
43+
if (detectCycles && source !== null && __alreadySeen.has(source)) {
3644
return __alreadySeen.get(source)
3745
}
3846

39-
if (isObservableArray(source)) {
47+
if (isObservableArray(source) || Array.isArray(source)) {
4048
const res = cache(__alreadySeen, source, [] as any, options)
4149
const toAdd = source.map(value => toJSHelper(value, options!, __alreadySeen))
4250
res.length = toAdd.length
4351
for (let i = 0, l = toAdd.length; i < l; i++) res[i] = toAdd[i]
4452
return res
4553
}
4654

47-
if (isObservableObject(source)) {
48-
const res = cache(__alreadySeen, source, {}, options)
49-
keys(source) // make sure we track the keys of the object
50-
for (let key in source) {
51-
res[key] = toJSHelper(source[key], options!, __alreadySeen)
52-
}
53-
return res
54-
}
55-
56-
if (isObservableMap(source)) {
55+
if (isObservableMap(source) || Object.getPrototypeOf(source) === Map.prototype) {
5756
if (options.exportMapsAsObjects === false) {
5857
const res = cache(__alreadySeen, source, new Map(), options)
5958
source.forEach((value, key) => {
@@ -69,9 +68,13 @@ function toJSHelper(source, options: ToJSOptions, __alreadySeen: Map<any, any>)
6968
}
7069
}
7170

72-
if (isObservableValue(source)) return toJSHelper(source.get(), options!, __alreadySeen)
71+
// Fallback to the situation that source is an ObservableObject or a plain object
72+
const res = cache(__alreadySeen, source, {}, options)
73+
for (let key in source) {
74+
res[key] = toJSHelper(source[key], options!, __alreadySeen)
75+
}
7376

74-
return source
77+
return res
7578
}
7679

7780
/**
@@ -81,15 +84,16 @@ export function toJS<T>(source: T, options?: ToJSOptions): T
8184
export function toJS(source: any, options?: ToJSOptions): any
8285
export function toJS(source, options: ToJSOptions) // internal overload
8386
export function toJS(source, options?: ToJSOptions) {
84-
if (!isObservable(source)) return source
85-
8687
// backward compatibility
8788
if (typeof options === "boolean") options = { detectCycles: options }
8889
if (!options) options = defaultOptions
89-
const detectCycles = options.detectCycles === true
90+
options.detectCycles =
91+
options.detectCycles === undefined
92+
? options.recurseEverything === true
93+
: options.detectCycles === true
9094

9195
let __alreadySeen
92-
if (detectCycles) __alreadySeen = new Map()
96+
if (options.detectCycles) __alreadySeen = new Map()
9397

9498
return toJSHelper(source, options, __alreadySeen)
9599
}

test/base/tojs.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,3 +341,66 @@ test("json cycles when exporting maps as maps", function() {
341341
expect(cloneD.get("c")).toBe(cloneC)
342342
expect(cloneA.e).toBe(cloneA)
343343
})
344+
345+
describe("recurseEverything set to true", function() {
346+
test("prototype chain will be removed even if the object is not observable", function() {
347+
function Person() {
348+
this.firstname = "michel"
349+
this.lastname = "weststrate"
350+
}
351+
const p = new Person()
352+
353+
expect(mobx.toJS(p)).toBeInstanceOf(Person)
354+
expect(mobx.toJS(p, { recurseEverything: true })).not.toBeInstanceOf(Person)
355+
expect(mobx.toJS(p)).toEqual({ firstname: "michel", lastname: "weststrate" })
356+
expect(mobx.toJS(p)).toEqual(mobx.toJS(p, { recurseEverything: true }))
357+
})
358+
359+
test("properties on prototype should be flattened to plain object", function() {
360+
const observableValue = mobx.observable.box("b")
361+
const Base = function() {
362+
this.a = "a"
363+
}
364+
const derived = Object.create(new Base(), {
365+
b: { value: observableValue, enumerable: true }
366+
})
367+
368+
const simpleCopy = mobx.toJS(derived)
369+
const deepCopy = mobx.toJS(derived, { recurseEverything: true })
370+
expect(simpleCopy).toBeInstanceOf(Base)
371+
expect(simpleCopy).toEqual({ b: observableValue })
372+
expect(simpleCopy.a).toBe("a")
373+
expect(simpleCopy.hasOwnProperty("a")).toBeFalsy()
374+
375+
expect(deepCopy).not.toBeInstanceOf(Base)
376+
expect(deepCopy).toEqual({ a: "a", b: "b" })
377+
expect(deepCopy.hasOwnProperty("a")).toBeTruthy()
378+
})
379+
380+
test("Date type should not be converted", function() {
381+
const date = new Date()
382+
expect(mobx.toJS(mobx.observable.box(date), { recurseEverything: true })).toBe(date)
383+
})
384+
385+
describe("observable array", function() {
386+
test("observable array should be converted to a plain array", function() {
387+
const arr = [1, 2, 3]
388+
expect(mobx.toJS(mobx.observable.array(arr), { recurseEverything: true })).toEqual(arr)
389+
expect(mobx.toJS(arr, { recurseEverything: true })).toEqual(arr)
390+
})
391+
392+
test("observable array inside an array will be converted with recurseEverything flag", function() {
393+
const obj = { arr: mobx.observable.array([1, 2, 3]) }
394+
expect(mobx.isObservable(mobx.toJS(obj).arr)).toBeTruthy()
395+
expect(mobx.isObservable(mobx.toJS(obj, { recurseEverything: true }).arr)).toBeFalsy()
396+
expect(mobx.toJS(obj, { recurseEverything: true }).arr).toEqual([1, 2, 3])
397+
})
398+
})
399+
400+
test("detectCycles should forcibly be set to true if recurseEverything is true", function() {
401+
const cycledObj = {}
402+
cycledObj.cycle = cycledObj
403+
const convertedObj = mobx.toJS({ key: cycledObj }, { recurseEverything: true })
404+
expect(convertedObj.key).toBe(convertedObj.key.cycle)
405+
})
406+
})

0 commit comments

Comments
 (0)