Front-Matter
I recently ran into a bug with my code that I figured was a good showcase for how this
works in Javascript. Before we dive in though, I need to give some context for the app.
Video Option
If you learn by watching instead of reading, check out the companion video below. If not, keep reading!
The App
I built/manage the app behind mortraqr.com. It's an app used by Mortuary School students across the country to keep track of their off-site training. Whenever a student does off-site training, they have to get it approved by their off-site instructor, referred to as a Preceptor.
It uses the following tech:
- Node/Express backend
- HTML rendered via Pug
- MongoDB with Mongoose
The Model for the Task
There is a lot more than this to the Model, but these are the relevant parts for this discussion. A Task
has an embedded updateHistory
Array that holds all updates.
// Task Model
{
"_id":"someid",
...moredata
"preceptor":"name@email.com",
"updateHistory":[
{
"_id":"someid",
"description":"update",
"type":"update",
},
],
}
Whenever a task is updated, a new entry is pushed into the updateHistory
array via a pre-save hook with Mongoose.
Broken Functionality
First off, here's the old code. There's a lot going on here, but we're really only concerned with a couple of parts.
All the code
// pre save hook for findOneAndUpdate() and findByIdAndUpdate()
taskSchema.pre('findOneAndUpdate', async function(next) {
// getUpdate() method provided via the Mongoose Query object
const updates = this.getUpdate();
// access the specific id for the task we are working on
const taskId = this._conditions._id;
// get the preceptor value from the getUpdate function
const preceptorUpdate = this.getUpdate().$set.preceptor;
// see if there is a preceptor value before moving on
if (updates.$set.preceptor) {
// get the preceptor value from old document
await this.findOne({ _id: taskId }, function(err, doc) {
const oldPreceptor = doc.preceptor;
const newPreceptor = preceptorUpdate != oldPreceptor ? true : false;
// see if the preceptor is changed
if (preceptorUpdate && newPreceptor) {
// concatatenate the new preceptor on the update change that is queued to
// get pushed onto the object
const newUpdate = this.getUpdate().$push.updateHistory.description.concat(
` Changes: Preceptor changed to ${preceptorUpdate}`
);
// set the update to our new value
this.getUpdate().$push.updateHistory.description = newUpdate;
}
});
}
// go to the next middleware
next();
});
The pre-save hook.
This allows us to manipulate the update before it saves to the DB. At this level, this
refers to the model.Query
. This provides us some methods and all the data for our updates.
// pre save hook for findOneAndUpdate() and findByIdAndUpdate()
taskSchema.pre('findOneAndUpdate', async function(next) {
// getUpdate() method provided via the Mongoose Query object
const updates = this.getUpdate();
// access the specific id for the task we are working on
const taskId = this._conditions._id;
// get the preceptor value from the getUpdate function
const preceptorUpdate = this.getUpdate().$set.preceptor;
// go to the next middleware
next();
});
My code - step by step
I first want to check to make sure there is a preceptor value (there isn't always) that is being updated.
if (updates.$set.preceptor) {
// ...more code
}
If there is, we have to get the original preceptor to see if it's different from the one we are trying to change. We have to go fetch the old data from the DB first.
// get the preceptor value from the old document
await this.findOne({ _id: taskId }, function(err, doc) {
const oldPreceptor = doc.preceptor;
const newPreceptor = preceptorUpdate != oldPreceptor ? true : false;
// more code...
});
Then verify there is an update and it's different from the existing one.
if (preceptorUpdate && newPreceptor) {
// concatatenate the new preceptor on the update change that is queued to
// get pushed onto the object
const newUpdate = this.getUpdate().$push.updateHistory.description.concat(
` Changes: Preceptor changed to ${preceptorUpdate}`
);
// set the update to our new value
this.getUpdate().$push.updateHistory.description = newUpdate;
}
First issue: function()
So far, everything has been going smoothly...but we just ran into our first piece of crap code. Damn you past Jared!
If we try to run it we get:
TypeError: Cannot read property '$push' of undefined
Why it broke
The reason it broke, depends on our understanding of how this
is binded to a function. When we use a function()
call in Node this
refers to the global, which means we longer have access to the model.Query
object with all it's handy methods.
The solve
If we dig into our toolbox and pull out a handy arrow function, we now have access to model.Query
again.
New Code:
await this.findOne({ _id: taskId }, (err, doc) => {
const oldPreceptor = doc.preceptor;
const newPreceptor = preceptorUpdate != oldPreceptor ? true : false;
if (preceptorUpdate && newPreceptor) {
const newUpdate = this.getUpdate().$push.updateHistory.description.concat(
` Changes: Preceptor changed to ${preceptorUpdate}`,
);
// set the update to our new value
this.getUpdate().$push.updateHistory.description = newUpdate;
}
});
And voila, it works!
Why does that work?
For those unfamiliar with arrow functions. They are just regular functions, but with a couple of differences:
- They have implicit returns
- They don't rebind the value of
this
. It bringsthis
from the scope above it.
Caveat
You might be thinking,
But...updates is defined at the top of the .pre function...why can't you just use that?
I gotta be honest, I'm not entirely sure. I assume it's because it is 3 levels deep, making it inaccessible. If you have a better explanation for this, I'm happy to hear.
Final Thoughts
This has been a fine example of "Jared doesn't know how Javascript works." Self-deprecation aside, I assume we as developers run into issues all the time due to the little intricacies of JS. I wanted to share a small moment of failure on my part.
If you have any suggestions on how to improve the code, or any general suggestions, leave a comment below!
As always, happy coding.
Top comments (2)
Amazing stuff! I always love reading how JS confuses people, just because I've been in so many confused situations before :D
Thanks! Yeah, it's nice to hear that we're not alone in our frustration haha. Thanks for reading!