Skip to content

fix(apiOptions): invalid computed option #225

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

Merged
merged 6 commits into from
Oct 14, 2019
Merged

fix(apiOptions): invalid computed option #225

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ae416c8
fix: invalid option
a631807682 Oct 12, 2019
2cfb1e5
fix: warning in dev mode only
a631807682 Oct 12, 2019
27c8077
fix: waring assigning to computed with no setter
a631807682 Oct 12, 2019
8cc601f
refactor: rename test description
a631807682 Oct 12, 2019
16aa305
refactor: change to cleaner code
a631807682 Oct 14, 2019
df16d31
refactor: cleanup not need type change
a631807682 Oct 14, 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
37 changes: 37 additions & 0 deletions packages/runtime-core/__tests__/apiOptions.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,5 +538,42 @@ describe('api: options', () => {

expect('Invalid watch option: "foo"').toHaveBeenWarned()
})

test('computed with setter and no getter', () => {
Copy link
Member

Choose a reason for hiding this comment

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

I think the previous description is a bit better describing what the test is

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 think the previous description is a bit better describing what the test is

but it wrapped in warnings describe, so i have no idea which is better.

const Comp = {
computed: {
foo: {
set() {}
}
},
render() {}
}

const root = nodeOps.createElement('div')
render(h(Comp), root)
expect('Getter is missing for computed property "foo"').toHaveBeenWarned()
})

test('assigning to computed with no setter', () => {
let instance: any
const Comp = {
computed: {
foo: {
get() {}
}
},
mounted() {
instance = this
},
render() {}
}

const root = nodeOps.createElement('div')
render(h(Comp), root)
instance.foo = 1
expect(
'Computed property "foo" was assigned to but it has no setter'
).toHaveBeenWarned()
})
})
})
38 changes: 27 additions & 11 deletions packages/runtime-core/src/apiOptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,12 @@ export type ComponentOptions =
type LegacyComponent = ComponentOptions

export interface ComputedOptions {
[key: string]:
| Function
| {
get: Function
set: Function
}
[key: string]: Function | ComputedOptionHandler
}

type ComputedOptionHandler = {
get: Function
set: Function
}

export interface MethodOptions {
Expand Down Expand Up @@ -245,12 +245,28 @@ export function applyOptions(
if (computedOptions) {
for (const key in computedOptions) {
const opt = (computedOptions as ComputedOptions)[key]
renderContext[key] = isFunction(opt)
? computed(opt.bind(ctx))
: computed({
get: opt.get.bind(ctx),
set: opt.set.bind(ctx)

if (isFunction(opt)) {
renderContext[key] = computed((opt as Function).bind(ctx))
} else {
const { get, set } = opt as ComputedOptionHandler
if (isFunction(get)) {
renderContext[key] = computed({
get: get.bind(ctx),
set: isFunction(set)
? set.bind(ctx)
: () => {
if (__DEV__) {
warn(
`Computed property "${key}" was assigned to but it has no setter`
)
}
}
})
} else if (__DEV__) {
warn(`Getter is missing for computed property "${key}"`)
}
}
}
}
if (methods) {
Expand Down