Skip to content

Commit

Permalink
Merge pull request #1511 from quanganhtran/master
Browse files Browse the repository at this point in the history
Throw error in development when @observable is used on getters
  • Loading branch information
mweststrate authored May 3, 2018
2 parents 6b50a8c + 8cca76d commit b0ee838
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/api/computed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const computedDecorator = createPropDecorator(
) => {
const { get, set } = descriptor // initialValue is the descriptor for get / set props
// Optimization: faster on decorator target or instance? Assuming target
// Optimiziation: find out if declaring on instance isn't just faster. (also makes the property descriptor simpler). But, more memory usage..
// Optimization: find out if declaring on instance isn't just faster. (also makes the property descriptor simpler). But, more memory usage..
const options = decoratorArgs[0] || {}
defineComputedProperty(instance, propertyName, { ...options, get, set })
}
Expand Down
8 changes: 7 additions & 1 deletion src/api/observabledecorator.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { defineObservableProperty } from "../types/observableobject"
import { fail } from "../utils/utils"
import { fail, invariant } from "../utils/utils"
import { IEnhancer } from "../types/modifiers"
import { createPropDecorator, BabelDescriptor } from "../utils/decorators2"

Expand All @@ -18,6 +18,12 @@ export function createDecoratorForEnhancer(enhancer: IEnhancer<any>): IObservabl
_decoratorTarget,
decoratorArgs: any[]
) => {
if (process.env.NODE_ENV !== "production") {
invariant(
!descriptor || !descriptor.get,
`@observable cannot be used on getter (property "${propertyName}"), use @computed instead.`
)
}
const initialValue = descriptor
? descriptor.initializer ? descriptor.initializer.call(target) : descriptor.value
: undefined
Expand Down
2 changes: 1 addition & 1 deletion src/utils/decorators2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export function createPropDecorator(
descriptor: BabelDescriptor | undefined,
applyImmediately?: any
// This is a special parameter to signal the direct application of a decorator, allow extendObservable to skip the entire type decoration part,
// as the instance to apply the deorator to equals the target
// as the instance to apply the decorator to equals the target
) {
if (applyImmediately === true) {
propertyCreator(target, prop, descriptor, target, decoratorArguments)
Expand Down
17 changes: 17 additions & 0 deletions test/base/decorate.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,3 +351,20 @@ test("decorate should work with ES6 constructor", () => {
title: observable
})
})

test("decorate should not allow @observable on getter", function() {
const obj = {
x: 0,
get y() {
return 0
}
}

decorate(obj, {
x: observable,
y: observable
})

expect(() => obj.x).toThrow(/"y"/)
expect(() => obj.y).toThrow()
})

0 comments on commit b0ee838

Please sign in to comment.