Skip to content

Enabled firebase to be called as function, thus enabling patterns where Firebase ref's are stored in root component #25

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

Merged
merged 13 commits into from
Sep 9, 2016
327 changes: 0 additions & 327 deletions dist/vuefire.js

This file was deleted.

1 change: 0 additions & 1 deletion dist/vuefire.min.js

This file was deleted.

1 change: 1 addition & 0 deletions src/vuefire.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ function ensureRefs (vm) {
var init = function () {
var bindings = this.$options.firebase
if (!bindings) return
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 it makes sense to move this one line below, so if the function is returning nothing we can skip too

if (typeof bindings === 'function') bindings = bindings.call(this)
ensureRefs(this)
for (var key in bindings) {
bind(this, key, bindings[key])
Expand Down
34 changes: 33 additions & 1 deletion tests/vuefire.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ var firebaseApp = Firebase.initializeApp({

describe('VueFire', function () {
var firebaseRef
var firebaseDb

beforeEach(function (done) {
firebaseRef = firebaseApp.database().ref()
firebaseDb = firebaseApp.database()
Copy link
Member

Choose a reason for hiding this comment

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

This can be moved inside the it function as it's only used there

firebaseRef = firebaseDb.ref()
firebaseRef.remove(function (error) {
if (error) {
done(error)
Expand All @@ -25,6 +27,36 @@ describe('VueFire', function () {
})
})

describe('is callable as Function', function () {
Copy link
Member

Choose a reason for hiding this comment

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

The title is missing something about option, which is what you're actually changing. I'd name it something like support Function options

it('returns correct ref on function call', function (done) {
var ChildComponent = Vue.extend({
name: 'ChildComponent',
firebase: function () {
Copy link
Member

Choose a reason for hiding this comment

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

Can't you just use this inside the Vue instance? Wouldn't that be easier?

expect(this.$root.database).to.deep.equal(firebaseDb)
done()
},
template: '<div>test</div>'
})
new Vue({
data: function () {
return {
database: firebaseDb
}
},
ready: function () {
Copy link
Member

Choose a reason for hiding this comment

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

The ready hook is unnecessary

expect('poop').to.equal('notpoop')
},
components: {
'child-component': ChildComponent
},
template: '<div><child-component></child-component></div>'
}).$mount()
Vue.nextTick(function () {
Copy link
Member

Choose a reason for hiding this comment

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

This is also unnecessary

Copy link
Author

Choose a reason for hiding this comment

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

Damn, I seem to not have removed my initial attempts at getting the test to work. I'll fix that ASAP.


})
})
})

describe('bind as Array', function () {
it('throws error for invalid firebase ref', function () {
helpers.invalidFirebaseRefs.forEach(function (ref) {
Expand Down