Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.1 release discussion #338

Closed
mbest opened this issue Feb 22, 2012 · 63 comments
Closed

2.1 release discussion #338

mbest opened this issue Feb 22, 2012 · 63 comments
Milestone

Comments

@mbest
Copy link
Member

mbest commented Feb 22, 2012

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.

@mbest
Copy link
Member Author

mbest commented Feb 22, 2012

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.

@mbest
Copy link
Member Author

mbest commented Feb 23, 2012

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

@SteveSanderson
Copy link
Contributor

Fantastic - thanks for this. Will look through it in detail tomorrow morning.

On 22 Feb 2012, at 22:52, Michael Best
reply@reply.github.com
wrote:

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.


Reply to this email directly or view it on GitHub:
#338

@rniemeyer
Copy link
Member

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).

@SteveSanderson
Copy link
Contributor

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.

@rniemeyer
Copy link
Member

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.

@mbest
Copy link
Member Author

mbest commented Feb 24, 2012

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

@rniemeyer
Copy link
Member

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

@mbest
Copy link
Member Author

mbest commented Feb 25, 2012

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.

@mbest
Copy link
Member Author

mbest commented Feb 25, 2012

@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.

@mbest
Copy link
Member Author

mbest commented Feb 25, 2012

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

@mbest
Copy link
Member Author

mbest commented Feb 26, 2012

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.

@vamp
Copy link

vamp commented Feb 27, 2012

What about #312 (comment) ?

@mbest
Copy link
Member Author

mbest commented Feb 29, 2012

Here's what's left:

@vamp
Copy link

vamp commented Feb 29, 2012

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

@mbest
Copy link
Member Author

mbest commented Feb 29, 2012

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.

@rniemeyer
Copy link
Member

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.

@mbest
Copy link
Member Author

mbest commented Mar 7, 2012

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.

@SteveSanderson
Copy link
Contributor

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.

@samdelagarza
Copy link

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

@samdelagarza
Copy link

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

@mbest
Copy link
Member Author

mbest commented Mar 7, 2012

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.

@mbest
Copy link
Member Author

mbest commented Mar 9, 2012

Thanks all, especially to Michael and Ryan.

You're welcome. Glad to have you back!

@mbest
Copy link
Member Author

mbest commented Mar 19, 2012

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?

@rniemeyer
Copy link
Member

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

@mbest
Copy link
Member Author

mbest commented Mar 20, 2012

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.

@SteveSanderson
Copy link
Contributor

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).

@mbest
Copy link
Member Author

mbest commented Mar 20, 2012

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

@SteveSanderson
Copy link
Contributor

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

Whoops... never mind

@SteveSanderson
Copy link
Contributor

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!

@SteveSanderson
Copy link
Contributor

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.

@mbest
Copy link
Member Author

mbest commented May 1, 2012

How do you build the documentation to test it?

@rniemeyer
Copy link
Member

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.

@mbest
Copy link
Member Author

mbest commented May 2, 2012

It works fine to install Jekyll

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

@rniemeyer
Copy link
Member

@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.

@mbest
Copy link
Member Author

mbest commented May 2, 2012

$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.

@mbest
Copy link
Member Author

mbest commented May 2, 2012

I can work on item 2.

@SteveSanderson
Copy link
Contributor

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/

@SteveSanderson
Copy link
Contributor

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?

@rniemeyer
Copy link
Member

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.

@mbest
Copy link
Member Author

mbest commented May 7, 2012

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.

@SteveSanderson
Copy link
Contributor

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...).

@SteveSanderson
Copy link
Contributor

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

@mbest mbest reopened this May 8, 2012
@mbest
Copy link
Member Author

mbest commented May 8, 2012

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.

@rniemeyer
Copy link
Member

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.

@mbest
Copy link
Member Author

mbest commented May 8, 2012

@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.

@mbest
Copy link
Member Author

mbest commented May 8, 2012

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.

@mbest mbest closed this as completed May 8, 2012
@rniemeyer
Copy link
Member

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

@mbest
Copy link
Member Author

mbest commented May 8, 2012

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.

@rniemeyer
Copy link
Member

@mbest
Copy link
Member Author

mbest commented May 8, 2012

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

@rniemeyer
Copy link
Member

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

@mbest
Copy link
Member Author

mbest commented May 9, 2012

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

@SteveSanderson
Copy link
Contributor

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

> But the big change you made is in how I described "binding context"

Very sorry if I acted too hastily there. I'm very happy to reconsider how
this is described and to go back to the explanation strategy you had in
mind if that is clearer. My concept of "binding context" is, as Ryan
described in his reply, the thing that holds $data, $parent, etc. If you
would like to reopen this discussion please say so - it may be early next
week when I'm back from Seattle when I can make time to amend the docs
further though.

Part of the reason I was keen to move quickly on the release, when it
seemed all the pieces were in place, was that I was doing a podcast about
Knockout on http://javascriptjabber.com/ this morning and wanted to be able
to say "yes, 2.1 is out right now" instead of "it's coming sometime soon".

Ryan, great to see the new blog design and the post about 2.1 appearing so
quickly!

Steve

On 8 May 2012 19:21, Michael Best <
reply@reply.github.com

wrote:

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


Reply to this email directly or view it on GitHub:
#338 (comment)

@mbest
Copy link
Member Author

mbest commented May 9, 2012

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?

@SteveSanderson
Copy link
Contributor

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.

@mbest
Copy link
Member Author

mbest commented May 14, 2012

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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants