Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

2.1 release discussion #338

Closed
mbest opened this Issue · 63 comments

5 participants

Michael Best Steven Sanderson Ryan Niemeyer denis Sam De La Garza
Michael Best
Collaborator

I went through all the open issues and selected 14 of them to include in 2.1. I've divided them using labels into bugs (3), documentation changes (2), enhancements (8), and cosmetic changes (1). I've also divided the code (non documentation) changes by impact: minor (8) and moderate (4). All but three of the issues already have code attached (some I just added today) or don't involve code changes.

Michael Best
Collaborator

I set up a 2.1.0 milestone that we can use to track the issues. All the issues also have a v2.1 label.

Michael Best
Collaborator

You can make a regular issue into a pull request by using the Github API or the hub tool. See this discussion on Stack Overflow for more information.

I use the API method like this:

curl -d "pull[base]=master" \
    -d "pull[head]=54-support-xhtml" \
    -d "pull[issue]=54" \
    -u "mbest:*******" \
    https://github.com/api/v2/json/pulls/SteveSanderson/knockout
Steven Sanderson
Ryan Niemeyer
Collaborator

Looks like a good list of fixes to me. For #182, I haven't looked at its current state in detail, but it would be nice if there is not a performance impact when $index is not required (which is normally the case).

Steven Sanderson

Nice, thanks for compiling this list.

I'd be completely happy for us to merge in the following immediately:
#334 - Leading spaces in CSS binding
#323 - class/className
#273 - select box flickering (with my update, if you agree with this)
#268 - implementation added
#267 - docs for AMD, but cherry-picking revision 83da03c only - i.e., excluding subsequent commits about running multiple versions of KO concurrently

Michael, if you have capacity to merge these items into master, please go ahead whenever you are ready (and if you are happy with my pull request for #268). Otherwise I can do them soon.

I'll continue looking at the following items on the list as soon as I can.

Ryan Niemeyer
Collaborator

I think that #323 could use a little more research. The current fix does not work properly for IE8 in standards mode and we should probably at least consider doing the same with for for/htmlFor.

Michael Best
Collaborator

@rniemeyer - I updated the fix for #323 to support IE8 standards, and for.

Ryan Niemeyer
Collaborator

@mbest - great! fix looks for #323 look good to go.

Michael Best
Collaborator

I merged #334 and #268 and will work on #267 shortly.

I've made further changes to #323 and #273, so please look those over.

Michael Best
Collaborator

@rniemeyer

For #182, I haven't looked at its current state in detail, but it would be nice if there is not a performance impact when $index is not required (which is normally the case).

I did some tests and found a slight performance hit (but not sure since performance measures of Javascript can vary a lot). I made some performance changes in the latest commit that bring it to equal (or maybe better) terms with the base version.

Michael Best
Collaborator

I'll work on the code for #139 and #122 today.

Michael Best
Collaborator

I pushed code for #122 (Internet Explorer and AutoComplete). The code works (I used the HTML provided in the issue to test it). I wanted it to work just like the default behavior, but it's a little different. Generally, the blur and change events happen at the same time (one after the other), but blur events also happen when the user switches to another window or application.

denis

What about #312 (comment) ?

Michael Best
Collaborator

Here's what's left:

  • #153 (iframe support) - The code needs to be cleaned up (spacing issues) and tested. I looked through the code briefly and nothing stood out as being wrong, but it should be thoroughly tested.
  • #54 (xhtml issues) - We have some unfinished discussion on whether to use upper or lower case for element tag names when doing comparisons. Steve favors upper case since it could be left unconverted for HTML elements. I favor lower case since I'd like it to match the lower case tag names for calls to createElement and getElementsByTagName and we could skip the comparison for XHTML elements.
  • #122 (IE autocomplete) - This is basically done, but I have an open question for Steve about a compatibility issue.
  • #182 (foreach index support) - Steve and Ryan, please check what I have done for this and give feedback.
  • #258 (documentation) - This is pretty minor and just needs to be done.
  • #260 (remove trailing whitespace) - I added some code to the build scripts to report which files have trailing white space. We could even extend it to have the scripts clean the files directly. The final commit for this will include updating all the code to remove the whitespace.
denis

What about performance fixes in 2.1:

regex for trim (current regex is not optimized):
http://jsperf.com/trim-performance-test

ieVersion enhancements:
http://jsperf.com/ie-version-test

Michael Best
Collaborator

What about performance fixes in 2.1:

Performance is, of course, important. If you think a change could be valuable in Knockout, please open an issue or pull request.

For 2.1, we've already included a number of performance updates:

#287 - memory leak fix
#272 - use cloneNode for inline templates
#245 - cache the parsed bindings
#249 - optimization of computed (only dispose inactive subscriptions)

And there are more.

Ryan Niemeyer
Collaborator

I can handle #258 at some point in the near future. I will also try to take a fresh look at #182, when I get a chance.

Michael Best
Collaborator

I added #361 regarding changing bindingContext.extend a bit. Since bindingContext.extend is new in 2.1, this is the time to get it right.

Steven Sanderson

Great stuff, everyone! For most of the remaining issues, I think we can have them cleared out this week (no travel for me this week - at last!).

The big ones, #153 and #182, will require some careful investigation and if they look like they will need further iterations we could consider deferring them to v2.2 (just to keep the release momentum high) but I do hope we can get them into v2.1.

Thanks all, especially to Michael and Ryan.

Sam De La Garza

Do you have a targeted release date for 2.1? The performance enhancements alone are very compelling.

Sam De La Garza

By the way is there a 2.1 branch that I can already explore? Or is it all getting put into the master?

Michael Best
Collaborator

By the way is there a 2.1 branch that I can already explore? Or is it all getting put into the master?

The master branch is what will become 2.1.

Michael Best
Collaborator

Thanks all, especially to Michael and Ryan.

You're welcome. Glad to have you back!

Michael Best
Collaborator

I can handle #258 at some point in the near future. I will also try to take a fresh look at #182, when I get a chance.

@rniemeyer - It's been almost three weeks. Any chance you could take care of these?

Ryan Niemeyer
Collaborator

@mbest - sorry, my time is very limited at the moment. Will do my best.

Michael Best
Collaborator

I'm happy with the implementation for #182. The changes are quite simple and clear. And the performance overhead of creating a new object for each item is offset by not creating an extra binding context object for each item.

For #153, I was hoping @SteveSanderson would work on it first, but I can do it if he's busy.

I took care of #258.

Steven Sanderson

Thanks for doing the doc update. I'll spend some time tomorrow morning looking at #153 and aim to clarify what exactly is involved in handling it (then possibly implement over the next few days).

Michael Best
Collaborator

@SteveSanderson - The two commits you made yesterday have "unknown" as the author: 440a933...5d9c406

Steven Sanderson

The two commits you made yesterday have "unknown" as the author

Whoops... never mind

Steven Sanderson

I'm going to be away next week, but I don't want to slow things down now we're this close. So, I took a quick decision to release the current state as 2.1.0beta - see the mailing list - hope you're all OK with this.

All that's left now is the cosmetic "remove trailing whitespace" issue (which I'm happy to do as soon as I'm back), and the cross-frame stuff. I'm not convinced that having extra params and code paths is justified to support cross-frame named templates, even with Michael's simplifications, especially because it's equally justified to prefer to load templates from the parent doc. I'm tending towards agreeing with Ryan that the core doesn't need to be opinionated about something this obscure. So, we may not have any other non-cosmetic changes to make before 2.1.0 is finished.

Michael - fantastic work with the $index support - that is really nice. Great to have that merged in.

I'll try to watch the mailing list for discussion about 2.1.0beta while I'm away, but will be slow to reply. Thanks all for your help!

Michael Best
Collaborator

@SteveSanderson

Thanks for taking care of the two "bugs". But your commits still are by "unknown". Must be something up with your git configuration.

So we just have #153 to put in, either implemented with #404 or #405 and then #260.

Steven Sanderson

But your commits still are by "unknown"

Whoops again. Thanks for reminding me. Fixed now.

So we just have #153 to put in, either implemented with #404 or #405 and then #260.

Yes - #153 is done now. I'm happy to do #260 tomorrow. Then I guess we can put a RC build out and update any affected docs before 2.1 is finalised :)

Steven Sanderson

OK, the release candidate is out, so all that's left is updating the docs. Tomorrow I'll write a list of what there is to update, then we can divide up the work. Then 2.1.0 is done :)

Thanks everyone.

Michael Best
Collaborator

Great! Thanks, Steve.

Michael Best
Collaborator

Do you think we should look at fixing #406 for 2.1.0? It appears that support for auto-complete fields (#122) doesn't work in IE9 because the propertychange event needs to be registered with attachEvent. Alternatively, the input event might work in IE9 instead of propertychange.

Michael Best
Collaborator

Tomorrow I'll write a list of what there is to update, then we can divide up the work.

What's the status of this?

Steven Sanderson

Sorry for the delay. I've been through the logs etc., and these are the doc updates I think we need before saying 2.1.0 is released:

  1. AMD support (already written, thanks to mtscout6)
  2. New context variables
    • $index - applies to the foreach binding
    • $parentContext - applies to the if, ifnot, with, and foreach bindings. Instead of duplicating the same content four times, should we have a new doc page called "context variables" and just link to it from all the control-flow binding pages?
  3. Enhancements for custom bindings
    • How to support virtual elements
    • How to add custom properties to binding contexts
  4. Minor utilities
    • The new ko.isComputed helper
    • Enhancements to ko.toJSON (the spacer/replacer options)
  5. Clarification about how computed observables handle circular bindings (we don't permit multiple concurrent evals for a particular computed).

Any volunteers to write particular ones? I'd be happy to take items 3 and 5. Ryan/Michael/anyone else keen to take particular ones?

I've just created a branch, gh-pages-2.1.0 (forked from gh-pages-2.0.1 which was never yet merged into gh-pages). Can we make the doc updates there? No need for pull requests - just commit changes to gh-pages-2.1.0, and then we can merge the whole lot back to gh-pages when it's all done.

Michael Best
Collaborator

How do you build the documentation to test it?

Ryan Niemeyer
Collaborator

It works fine to install Jekyll and build it locally. From the root directory running jekyll --pygments --safe is the equivalent of what github pages would do.

As for the docs, I am happy to help out. I will start on number 4. I also would like to create 2.0 versions of all of the fiddles here and link to them from the appropriate doc pages. Then, I can circle back and help with number 2, if it still needs help.

Michael Best
Collaborator

It works fine to install Jekyll

Hmm. Not sure I want to tackle that right now.

Ryan Niemeyer
Collaborator

@mbest - no problem. If you are inclined to write some docs, then I would be happy to help clean them up, if there are any issues with them.

Michael Best
Collaborator

$parentContext - applies to the if, ifnot, with, and foreach

Actually this doesn't apply to the if and ifnot bindings. But it does apply to template if foreach or data is given.

Michael Best
Collaborator

I can work on item 2.

Steven Sanderson

No pressure to install Jekyll, but just in case anyone wants the "fast track" steps for Jekyll on Windows, see here: http://www.madhur.co.in/blog/2011/09/01/runningjekyllwindows.html - note you don't need pygments, so you don't need Python either. Just Ruby will do.

Then, once you have jekyll installed:

cd folder\where\gh-pages\is
jekyll --server --auto

Then browse to http://localhost:4000/

Steven Sanderson

OK, just added the docs for #3 and #5. I see Ryan's already done #4.

As soon as #2 is in, does anyone see any reason why we shouldn't announce KO 2.1.0 is released?

Ryan Niemeyer
Collaborator

Sounds fine to me. I am still planning to get jsFiddle links for all of the samples, but am out of town for work at the moment. No need to wait on that one though. I was also thinking of adding a sample to the ko.toJSON part to show what you can do with the "replacer" argument. Nothing that needs to wait though.

Michael Best
Collaborator

For item 2, I've done the following:

  • created a new page about binding contexts and included documentation about $index and $parentContext
  • added links to the new page from various other pages that mention binding context
  • removed the list of special context properties from the foreach, with, and unobtrusive event handling pages
  • added a section to foreach about $index

Use this link to view the non-whitespace changes.

Ryan or Steve, please feel free to review what I've done and change/expand it as you see fit. Also it'll probably be good to add or expand examples to demonstrate $index and $parentContext.

Steven Sanderson

Thanks Michael! I've merged this all into the gh-pages branch now. Michael, hope you don't mind me making some tweaks to the phrasing, not that there was anything wrong with what you wrote, just that I was trying to make the tone more closely match other pages. You having produced the new doc page was the hard work so thanks for that. The editing only took a few minutes. I did add an example of $index.

OK, so we're done, and 2.1.0 is released! I've just:

  • Updated the KO source to add a v2.1.0 tag
  • Uploaded 2.1.0 to the "downloads" page on GitHub
  • Merged gh-pages-2.1.0 into gh-pages
  • Updated the homepage and installation page on knockoutjs.com to refer to 2.1.0

Would either of you like to post an announcement to the mailing list? I'm happy to do it myself, but I'd also like to give an opportunity for either of you to be the one bringing the good news this time, especially to emphasise that it's not just me producing all the great new features...!

To broadcast this further, I'll write a blog post highlighting the changes in the next week or so (or if Ryan beats me to it, I might just post a link to his...).

Steven Sanderson

Marking this issue as closed now, but please feel free to continue the discussion here for as long as it is relevant.

Michael Best mbest reopened this
Michael Best
Collaborator

Steve, I feel you moved a bit too soon (although I understand we've been waiting a while to release 2.1). But you undid changes I made without finding out why I had made them.

But the big change you made is in how I described "binding context". I spent a lot of time thinking about how to describe it while taking into account how the term was used already in the documentation (and the general definition of "context"). For example, the with binding documentation uses it just like I did:

The with binding creates a new binding context, so that descendant elements are bound in the context of a specified object.

I think that although we have an object called bindingContext, it's really a "context supplement," providing links and additional information about the context.

If we can, I'd like to change the document back so that "binding context" is used consistently to mean the view model object.

Ryan Niemeyer
Collaborator

I can announce this to the list, but since Michael really was the driving force behind this release, I would be happy to let him make the post as well. @mbest - let me know if you have time and want to, otherwise I will do it.

It sounds like the true needs to be adding back to the ko.applyBindingsToDescendants call.

As for binding context, I don't have a huge preference, but I generally think that binding context should refer to the object that contains $data, $parent, etc. (the same as bindingContext). I think that view model or data should be used for the actual data. I don't personally find it confusing to talk about using a "new binding context" to mean that the data will have a different view model, as it is implied that $data of the binding context is what you are binding against.

I will put out a blog post on 2.1 as well today or tomorrow.

Michael Best
Collaborator

@rniemeyer, thanks for the information. So instead of the view model being the binding context, it's the data of the binding context? I'll take another look through the documentation to see if I feel it presents this idea consistently.

Michael Best
Collaborator

It sounds like the true needs to be adding back to the ko.applyBindingsToDescendants call.

Oops. Steve is right. I was confused because applyBindingsToDescendantsInternal takes a third parameter which needs to be true, but applyBindingsToDescendants takes two and simply calls applyBindingsToDescendantsInternal with true as the third parameter. Sorry about that.

Michael Best mbest closed this
Ryan Niemeyer
Collaborator

@mbest - :) I had looked at it quick too and confused applyBindingsToDescendantsInternal with applyBindingsToDescendants as well.

Michael Best
Collaborator

I can announce this to the list, but since Michael really was the driving force behind this release, I would be happy to let him make the post as well. @mbest - let me know if you have time and want to, otherwise I will do it.

I'm happy to do it. I need to get some sleep now, but I'll send it off in the morning.

Michael Best
Collaborator

I'd like to clean up (delete) all the leftover branches. Any downside to that?

Ryan Niemeyer
Collaborator

I personally don't think that keeping the branches is valuable. I'd like to see them cleaned up too.

Michael Best
Collaborator

Other than the bold ones below, are there any of these we need to keep?

1.3
122-IE-autocomplete
139-expose-JSON-stringify-args
153-cross-frame-disposal
153-cross-frame-disposal-with-named-templates
182-foreach-index
216-template-in-textarea
225-observable-value-in-debugger
260-trailing-white-space
268-isComputed
273-select-flicker
323-IE-class-attr
361-extend-not-child-context
361-remove-useless-parent
387-recursive-computed-eval
406-propertychange-in-ie9
411-tagnamelower-check
413-arraymapping-tolerate-manual-edits
420-clean-inline-template
436_spec_fix_ie7_jquery
440-fix-rewritten-template-foreach
54-support-xhtml
consistent-builds
gh-pages
gh-pages-1.3.0
gh-pages-2.0.1
ie-fixes-for-2.1
master
mbest-external-containerless-bindings
mbest-named_inline_templates

Steven Sanderson
Michael Best
Collaborator

Nope, I think the others can all go. Thanks for tidying this.

Done.

Very sorry if I acted too hastily there.

I felt a bit put off, but it's good we got 2.1 released.

I'm very happy to reconsider how this is described...

I've read over your changes and the other related documentation, and I think your explanation is good too. I'm happy to keep it like it is.

I was doing a podcast about Knockout on http://javascriptjabber.com/ this morning.

I went to their site, but couldn't find the podcast. Do you have a link?

Steven Sanderson

Do you have a link?

We only just recorded it yesterday, so it may take a few days for them to finalise the production and upload it. I'm not sure how fast they typically do this.

Michael Best
Collaborator

Just listened to the podcast. Great to get more exposure for Knockout! I don't remember you mentioning version 2.1 ;-)

I thought I'd post a binding in one of my apps (not quite 14 but almost):

    <select name="stdName" data-bind="
        enabled: $parents[1].data.accessToCreateSchema,
        id: name,
        options: $parents[1].data.stdVariables,
        optionsCaption: '-- Select or New --',
        optionsValue: 'name',
        optionsText: 'name',
        optionsBind: 'attr.title: $parents[2].stdVarTitle($data)',
        value: stdName,
        valueUpdate: 'afterkeydown',
        setfocus: selectHasFocus"></select>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.