Skip to content

Fix #210 again.#222

Merged
dr-dimitru merged 1 commit intoveliovgroup:devfrom
exKAZUu:dev
Sep 7, 2016
Merged

Fix #210 again.#222
dr-dimitru merged 1 commit intoveliovgroup:devfrom
exKAZUu:dev

Conversation

@exKAZUu
Copy link
Copy Markdown
Contributor

@exKAZUu exKAZUu commented Sep 6, 2016

Fix #210 (the inconsistency of the behaviour of find and findOne between Mongo.Collection and FilesCollection)

@exKAZUu
Copy link
Copy Markdown
Contributor Author

exKAZUu commented Sep 6, 2016

Actually, I didn't test files.coffee directly because I'm not sure how to test a Meteor package. Sorry for that.

check selector, Match.OneOf Object, String, null
check options, Match.Optional Object

selector = {} unless arguments.length
Copy link
Copy Markdown
Member

@dr-dimitru dr-dimitru Sep 7, 2016

Choose a reason for hiding this comment

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

You're bringing back old behaviour unless arguments.length will be triggered on undefined, null, false all falsy values.

While find: (selector = {}) is equal to selector ?= {}, which will set selector only if it's null or undefined

Update: my bad, I was wrong, hold on for test from my end

@dr-dimitru dr-dimitru merged commit 0f37c75 into veliovgroup:dev Sep 7, 2016
@dr-dimitru
Copy link
Copy Markdown
Member

I've done tests. And it fails:

Code FilesCollection MongoCollection
find() Exception: Match.Failed LocalCollection.Cursor
find({}) FilesCursor LocalCollection.Cursor
find(null) FilesCursor LocalCollection.Cursor
find(false) Exception: Match.Failed LocalCollection.Cursor
find(true) Exception: Match.Failed Uncaught TypeError: Cannot use 'in' operator
find([]) Exception: Match.Failed Error: Invalid selector
find(12345) Exception: Match.Failed LocalCollection.Cursor
find('abcde') FilesCursor LocalCollection.Cursor
------ ----------------- -------------------
findOne() undefined undefined
findOne({}) undefined undefined
findOne(null) undefined undefined
findOne(false) undefined undefined
findOne(true) Uncaught TypeError: Cannot use 'in' operator Uncaught TypeError: Cannot use 'in' operator
findOne([]) Exception: Match.Failed Error: Invalid selector
findOne(12345) Exception: Match.Failed LocalCollection.Cursor
findOne('abcde') undefined undefined

@dr-dimitru
Copy link
Copy Markdown
Member

so, different behaviour in: findOne([]), findOne(12345), find(), find(false), find(true), find([]) and find(12345)

Will you fix it with one more PR?

@exKAZUu
Copy link
Copy Markdown
Contributor Author

exKAZUu commented Sep 7, 2016

Thank you for your detailed investigation.
I will try making another PR tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants