Fix unable to mapReduce with decorator on an Instance type #78
Conversation
…tion and add new tests
|
Changes Unknown when pulling c79b2a4 on RagibHasin:master into ** on SierraSoftworks:master**. |
There was a problem hiding this comment.
Looks mostly pretty good, I don't believe it'll break any existing code with the previous API, but can you confirm that quickly please? (will determine whether this is a patch or major version bump)
Otherwise, all looks like a nice improvement on the approach being used.
lib/Model.ts
Outdated
| return resolve(); | ||
| }); | ||
| } | ||
| else return reject("mapReduceOptions not provided"); |
There was a problem hiding this comment.
Can we reject this with an actual Error object? Strings as errors are generally a bad idea for debugging and handling purposes.
There was a problem hiding this comment.
Oops! 😛 My mistake. Just missed the new Error.
lib/Model.ts
Outdated
| if (err) return reject(err); | ||
| return resolve(); | ||
| }); | ||
| if (instanceType.mapReduceOptions) { |
There was a problem hiding this comment.
Can we rather shift this test up to the top of this function (line 733 or so) to make it easier to read?
There was a problem hiding this comment.
Of course. That will be cleaner.
|
Changes Unknown when pulling 5eb054f on RagibHasin:master into ** on SierraSoftworks:master**. |
|
Looks good, I'll go about merging and releasing this this evening if that works for you? Let me know if it's more urgent and I'll shuffle some stuff around. |
|
It is not very important but is good to get it released. I'm using my fork at this moment for a project. And it did not break any previous code of mine. |
|
Deployed in v7.2.1 - thanks for the PR @RagibHasin and as always, if you run into any issues please feel free to open an issue 👍 |
Sorry for knocking again.
I noticed that when applying mapReduce functions with decorators, it failed to compile. (Actually for the ugly static type checks I added at the last moment). So it seems better to me to check for those functions at runtime which will also be useful for JavaScript users. And added accompanying test.
Best regards.