search

jakearchibald / wittr

Silly demo app for an online course
385 stars 555 forks source link

this is being shadowed in _onSocketMessage in several branches. #39

Open jeffreydwalter opened 6 years ago

jeffreydwalter commented 6 years ago

In the initial branch, _onSocketMessage refers to this._postsView.addPosts(), which works fine. In later branches, this._postsView.addPosts() is wrapped in other promises and loses it's scope.

Here's one example in Lesson 4, step 7 (task-clean-db branch): https://github.com/jakearchibald/wittr/blob/43e3098c498f3691a8e47cf63ea0d46080f5d56a/public/js/main/IndexController.js#L174

The fix is to assign this to var indexController = this; as is being done in the rest of the code.

jeffreydwalter commented 6 years ago

I saw this in at least one other branch earlier but fixed it without really thinking to report it as a bug.

For clarity and consistency sake, you guys should probably just make that assignment in _onSocketMessage() (all the functions in IndexController really) from the very first lesson, even though it's not strictly needed, since you do that in most of the other functions.

jeffreydwalter commented 6 years ago

I created a PR to address this issue in task-clean-db. https://github.com/jakearchibald/wittr/pull/40

jeffreydwalter commented 6 years ago

The error message you see in the console is: image

jeffreydwalter commented 6 years ago

I left a post in the Udacity forums about this issue as well.

jeffreydwalter commented 6 years ago

Just realized this issue was caused by me moving this._postsView.addPosts(messages); inside this._dbPromise.then(function(db), which is clearly wrong.

I'd still argue that making the var indexController = this; assignment would be good form.