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

Fix array update proxy (important fix) #1985

Merged
merged 4 commits into from
Jun 9, 2019
Merged

Fix array update proxy (important fix) #1985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8935990
fix array proxy for update
xaviergonz Jun 4, 2019
650cff9
revert wrong formatting
xaviergonz Jun 4, 2019
6c1aa5b
revert wrong formatting
xaviergonz Jun 4, 2019
823670d
add unit test
xaviergonz Jun 5, 2019
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
2 changes: 1 addition & 1 deletion src/types/observablearray.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ const arrayExtensions = {
if (hasInterceptors(adm)) {
const change = interceptChange<IArrayWillChange<any>>(adm as any, {
type: "update",
object: this.proxy,
object: adm.proxy as any, // since "this" is the real array we need to pass its proxy
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be tested to make sure it doesn't happen again and to show what it fixed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unit test added

index,
newValue
})
Expand Down
9 changes: 9 additions & 0 deletions test/base/intercept.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ test("intercept observable value", () => {
expect(a.get()).toBe(3)

d = intercept(a, c => {
expect(c.object).toBe(a)
if (c.newValue % 2 === 0) {
throw "value should be odd!"
}
Expand All @@ -34,6 +35,7 @@ test("intercept observable value", () => {

d()
d = intercept(a, c => {
expect(c.object).toBe(a)
c.newValue *= 2
return c
})
Expand All @@ -42,6 +44,7 @@ test("intercept observable value", () => {
expect(a.get()).toBe(12)

intercept(a, c => {
expect(c.object).toBe(a)
c.newValue += 1
return c
})
Expand All @@ -64,6 +67,7 @@ test("intercept array", () => {
d()

d = intercept(a, c => {
expect(c.object).toBe(a)
if (c.type === "splice") {
c.added.push(c.added[0] * 2)
c.removedCount = 1
Expand All @@ -87,6 +91,7 @@ test("intercept object", () => {
})

intercept(a, change => {
expect(change.object).toBe(a)
change.newValue *= 3
return change
})
Expand Down Expand Up @@ -121,6 +126,7 @@ test("intercept object", () => {
test("intercept property additions", () => {
const a = m.observable({})
const d4 = intercept(a, change => {
expect(change.object).toBe(a)
if (change.type === "add") {
return null
}
Expand All @@ -144,6 +150,7 @@ test("intercept map", () => {
})

intercept(a, c => {
expect(c.object).toBe(a)
c.newValue *= 3
return c
})
Expand All @@ -161,6 +168,7 @@ test("intercept map", () => {
expect(a.get("b")).toBe(16)

const d3 = intercept(a, c => {
expect(c.object).toBe(a)
expect(c.name).toBe("b"), expect(c.object).toBe(a)
expect(c.type).toBe("update")
return null
Expand All @@ -174,6 +182,7 @@ test("intercept map", () => {
expect(a.get("b")).toBe(22)

const d4 = intercept(a, c => {
expect(c.object).toBe(a)
if (c.type === "delete") return null
return c
})
Expand Down