search

knockout / tko

🥊 Technical Knockout – The Monorepo for Knockout.js (4.0+)
http://www.tko.io
Other
275 stars 34 forks source link

🎥 Incorporate ongoing PRs from Knockout/knockout #5

Closed brianmhunt closed 6 years ago

brianmhunt commented 8 years ago

From: 

$ git log "v3.4.0"..HEAD --oneline

Complete

brianmhunt commented 7 years ago

@mbest – I've noticed a bunch of changes to KO 3.4 since the tko fork (good work, btw); could you please add a link to each of the PRs/merges here, so we can track them and they can be incorporated without having to search through the KO issues database. Otherwise I fear we may end up with some regressions from KO 3.4 to KO 4 (/TKO 1.).

Of course, where the change is apparent please feel free to incorporate directly.

Many thanks.

EDIT: I'm sure there's an automated way to get a list of all changes merged since 3.4.0. So this issue could be solved by figuring out the git command for that. :)

JD-Robbs commented 7 years ago

Like git log "v3.4.0"..HEAD? I.e. just a simple listing of commits (hash, author, ~data~ date and description) since the v3.4.0 tag.

brianmhunt commented 6 years ago

@mbest - Thanks for all these PRs; I'll be caught up to the latest ones soon.

A couple notes I wanted to share:

  1. Comments around new code have really helped; not just for their content, but especially because they generally provide unique searchable text that makes it easier to pare down where the changes are in the code-base.

  2. I'm getting a bizarre error using the with binding, notably if you look at tko's WithBindingHandler in with.js, and the WithBindingHandlerFailsInexplicably, there's something amiss in that the latter fails the test Should update "with" binding before descendant bindings in asyncBindingBehavior.js. However the topmost calls they make appear to be identical, and all other tests appear to pass. I dug down and there seem to be a lot more calls to the _evalIfChanged with the version that works. The essence of the problem occurring in the test appear to be that the observables are not being triggered in the correct order.

You can try it out by running npm run watch in the tko.binding.if directory, and compare them by renaming the export (i.e. changing WithBindingHandlerFailsInexplicably to WithBindingHandler and renaming the other so there's no name conflict).

I'll keep staring at it, but since it seems to be deep in the deferUpdate code I thought you might have some insight. . I was using createStaticChildContext instead of createChildContext