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

Release version 3.4.0 #1858

Closed
10 tasks done
mbest opened this issue Aug 27, 2015 · 61 comments
Closed
10 tasks done

Release version 3.4.0 #1858

mbest opened this issue Aug 27, 2015 · 61 comments
Milestone

Comments

@mbest
Copy link
Member

mbest commented Aug 27, 2015

Steps to release:

  • Run tests on supported browsers and fix any failing tests.
  • Update documentation with new features (see Documentation updates for 3.4.0 #1873)
  • Write release notes.
  • Publish release candidate.
  • Respond to any urgent bugs.
  • Build and create release in Github.
    • Update doc site with latest version.
  • Publish to NPM (@mbest)
  • Publish to NuGet (@SteveSanderson).
  • Post on Google groups, blogs, etc. about new release.
@mbest mbest added this to the 3.4.0 milestone Aug 27, 2015
@brianmhunt
Copy link
Member

Great list, thanks @mbest.

For ease of reference: The Travis-CI build from the #1360 branch, which has merged all the 3.4.0 changes.

The debug build from #1360 out to be near-bitwise-identical, and here are the errors reported on that branch:

IE 10.0.0 (Windows 8 0.0.0) registerEventHandler should not use jQuery eventing with useOnlyNativeEvents option set to true FAILED
    Expected false to be true.
    Expected false to be true.
IE 9.0.0 (Windows 7 0.0.0) registerEventHandler should not use jQuery eventing with useOnlyNativeEvents option set to true FAILED
    Expected false to be true.
    Expected false to be true.
IE 11.0.0 (Windows 8.1 0.0.0) registerEventHandler should not use jQuery eventing with useOnlyNativeEvents option set to true FAILED
    Expected false to be true.
    Expected false to be true.
IE 9.0.0 (Windows 7 0.0.0) Binding: TextInput Should update observable on input event (on supported browsers) or propertychange event (on old IE) FAILED
    Expected 123 to equal 'some user-entered value'.
IE 11.0.0 (Windows 8.1 0.0.0) Binding: Hasfocus Should not unnecessarily focus or blur an element that is already focused/blurred FAILED
    Expected true to equal false.
IE 8.0.0 (Windows 7 0.0.0) Binding attribute syntax Should not bind against text content inside restricted elements <template> FAILED
    Expected '<p>replaced</p><template>replaced</template><p>replaced</p>' to contain html '<p>replaced</p><template>test</template><p>replaced</p>'.
IE 8.0.0 (Windows 7 0.0.0) registerEventHandler should not use jQuery eventing with useOnlyNativeEvents option set to true FAILED
    Expected false to be true.
    Error: Unspecified error.
INFO [launcher.sauce]: internet explorer 7.0 (Windows XP) session at https://saucelabs.com/tests/1eed8aff50054d15856d9ea2e8f0fcdd
IE 8.0.0 (Windows 7 0.0.0) Binding: Hasfocus Should respond to changes on an observable value by blurring or focusing the element FAILED
    Expected true to equal false.
IE 8.0.0 (Windows 7 0.0.0) Binding: Hasfocus Should not unnecessarily focus or blur an element that is already focused/blurred FAILED
    Expected true to equal false.
INFO [IE 7.0.0 (Windows XP 0.0.0)]: Connected on socket ZluyP3ryyOVEPCTJZvLH with id 60607771
IE 7.0.0 (Windows XP 0.0.0) Binding attribute syntax Should not bind against text content inside restricted elements <template> FAILED
    Expected '<p>replaced</p><template>replaced</template><p>replaced</p>' to contain html '<p>replaced</p><template>test</template><p>replaced</p>'.
IE 7.0.0 (Windows XP 0.0.0) registerEventHandler should not use jQuery eventing with useOnlyNativeEvents option set to true FAILED
    Expected false to be true.
    Error: Unspecified error.
IE 7.0.0 (Windows XP 0.0.0) Parse HTML fragment should parse <thead><tr><th><thcomponent>hello</thcomponent></th></tr></thead> correctly FAILED
    Expected '<thead><tr><th><thcomponent>hello</thcomponent></th></tr></thead><tbody></tbody>' to contain html '<thead><tr><th><thcomponent>hello</thcomponent></th></tr></thead>'.
IE 7.0.0 (Windows XP 0.0.0) Binding: Foreach Exception in afterAdd callback should not cause extra elements on next update FAILED
    Expected 'A
    B
    C' to contain text 'ABC'.
    Expected 'A
    B
    C
    D' to contain text 'ABCD'.
    Expected 'A
    B
    C
    D
    E' to contain text 'ABCDE'.
IE 7.0.0 (Windows XP 0.0.0) Binding: Hasfocus Should not unnecessarily focus or blur an element that is already focused/blurred FAILED
    Expected true to equal false.

@mbest
Copy link
Member Author

mbest commented Aug 28, 2015

@brianmhunt, That's a good start. I fixed the useOnlyNativeEvents tests (see aae6bf0).

For these tests that fail when using jQuery,

Parse HTML fragment should parse <tr-component></tr-component> correctly.
Expected [ ] to equal [ '<tr-component></tr-component>' ].

Parse HTML fragment should parse <tbody-component>world</tbody-component> correctly.
Expected [ ] to equal [ '<tbody-component>world</tbody-component>' ].

Parse HTML fragment should parse <tfoot-component>foo</tfoot-component> correctly.
Expected [ ] to equal [ '<tfoot-component>foo</tfoot-component>' ].

it appears that jQuery doesn't parse these elements correctly (it tries to parse them as table elements). Knockout's parser handles these now (see #1773). Perhaps we shouldn't use jQuery for parsing anymore. Any thoughts?

@brianmhunt
Copy link
Member

@mbest Great about the useNativeEvents. I've merged the commit you linked into #1360, and the tests have run again.

On the parsing of HTML fragments, my personal preference would be to stop deferring to jQuery for parsing. The difficulties of working around the failings of jQuery in this case strike me as harder than writing a clean parser that works correctly. I am not aware of any particular upside to the jQuery parser – are you, @mbest?

The only risk I see is something we are not aware of that crops up because we're dropping support for jQuery HTML parsing but someone is using it in a live environment.

@brianmhunt
Copy link
Member

Incidentally, I think ko.onError should have its own page since we might be expanding the error handling - particularly with respect to bindings i.e. #1628 . It's not a strong preference, but I think it would be more consistent and easier if it had its own page.

@mbest
Copy link
Member Author

mbest commented Aug 28, 2015

Since the jQuery parsing problem happens only with elements containing a hyphen, a workaround could be to not use jQuery if the element name contains a hyphen. We could actually code this using a "feature detection" technique so that a working future version of jQuery can be used automatically.

@mbest
Copy link
Member Author

mbest commented Aug 28, 2015

I decided to go ahead and implement the above "fix". I also fixed an issue with parsing <option> elements in old IE (see 200f196).

@brianmhunt
Copy link
Member

Thanks @mbest – great stuff. I think that is the best option right now. I have merged 200f196 into #1360: the test results are ongoing, but so-far so good.

@rniemeyer
Copy link
Member

Some prelim release notes:

Enhancements

#1715 - Add ko.onError handler that provides more consistent error/stack information for async operations and event handlers
#1728 - Include deferred updates functionality in Knockout core
    #1738 - Deferred Updates 1 - ko tasks functionality; deferred extender for enabling deferred Updates
    #1753 - Deferred Updates 2 - add ability to turn on deferred updates globally by setting ko.options.deferUpdates = true;
    #1795 - Deferred Updates 3 - dependency tree scheduling
#1774 - Add ko.options.useOnlyNativeEvents (default false) that can control if KO uses jQuery (if available) for binding events
#1839 - Performance Improvement - for named templates that use a template or normal element use the element directly as the template to clone
#1840 - Performance Improvement - Make observables 2-3x faster to instantiate and reduce memory usage by 50%
#1841 - Performance Improvement - Make computeds 2x faster to instantiate and reduce memory usage by 50%
#1870 - Add ko.isPureComputed.

Fixes

#1256 - Better handling when an item is added while a beforeRemove animation is running
#1380 - observableArray sort and reverse methods now return the observableArray rather than the underlying array
#1510 - Do not define a custom element when a component name matches a standard element
#1591 - Make checked binding work properly with writable computed observables
#1613 - IE8 issue with calling .apply with an empty second argument
#1683 - Rearrange code to avoid prototype issue in Opera mobile
#1709 - dontLimitMoves option should be respected when comparing arrays
#1718 - ko.toJS no longer replaces RegExp objects with plain objects
#1756 - Use a hasOwnProperty check when registering components
#1781 - Trim string in default css binding
#1783 - Skip template elements when binding
#1788 - Fix textInput edge case in IE9
#1790 - Remove version from bower.json and don't update it in build
#1794 - Exception in an afterAdd callback should not cause duplicate elements
#1810 - Don't check for require for CommonJS
#1855 - Fix potential memory leak in component binding
#1893 - hasFocus fixes in IE

@mbest
Copy link
Member Author

mbest commented Aug 30, 2015

The list looks good, @rniemeyer.

I fixed some more tests that were failing in old IE (967389d):

  • Binding attribute syntax Should not bind against text content inside restricted elements <template>
  • Parse HTML fragment should parse <thead><tr><th><thcomponent>hello</thcomponent></th></tr></thead> correctly
  • Binding: Foreach Exception in afterAdd callback should not cause extra elements on next update

brianmhunt added a commit to brianmhunt/knockout that referenced this issue Aug 30, 2015
@brianmhunt
Copy link
Member

Great stuff. I think the list looks good too, @rniemeyer

@mbest – great job cleaning up the IE tests. I noticed a couple IE test failures still linger:

IE 9.0.0 (Windows 7 0.0.0) Binding: TextInput Should update observable on input event (on supported browsers) or propertychange event (on old IE) FAILED
    Expected 123 to equal 'some user-entered value'.
IE 11.0.0 (Windows 8.1 0.0.0) Binding: Hasfocus Should not unnecessarily focus or blur an element that is already focused/blurred FAILED
IE 8.0.0 (Windows 7 0.0.0) Binding: Hasfocus Should respond to changes on an observable value by blurring or focusing the element FAILED
    Expected true to equal false.
IE 8.0.0 (Windows 7 0.0.0) Binding: Hasfocus Should not unnecessarily focus or blur an element that is already focused/blurred FAILED
    Expected true to equal false.

@mbest
Copy link
Member Author

mbest commented Aug 31, 2015

The textInput issue is related to #1788. I fixed the test with e3e4b13.
The hasFocus issue appears to be caused by a bug in IE. This required a code change: 9090502.

@brianmhunt
Copy link
Member

Awesome, thanks @mbest.

The only other one that came up:

IE 10.0.0 (Windows 8 0.0.0) Tasks scheduler Should process tasks asynchronously FAILED
    Expected 1 to equal 2.

But, mind you, IE7/8 might not've run.

@mbest
Copy link
Member Author

mbest commented Aug 31, 2015

@brianmhunt I've run the tests in IE 10 and don't see any error, so I'm not sure why that error is showing up for you. I also don't see any errors in Firefox on my computer (using version 34).

@brianmhunt
Copy link
Member

@mbest The Firefox errors occur in versions 10 – 23, so you'll have to download an old release or set up SauceLabs or Browserstack to test it.

I don't have any more debugging info on the IE 10 fail, I'm afraid, other than it occurs. It seems unlikely, but perhaps there is a race condition that's only breached because the remote browser environment is slow. Just a stab in the dark, really.

@SteveSanderson
Copy link
Contributor

About this failure:

IE 10.0.0 (Windows 8 0.0.0) Tasks scheduler Should process tasks asynchronously FAILED
Expected 1 to equal 2.

I get that failure intermittently when running the tests in most versions of IE. It tends to happen the first time the browser is opened, but not afterwards. Since it appears to be a test timing issue (not a production code defect), I'm not concerned about blocking the release on it. But it will continue to interfere with the quality of automated test results so we'll probably want to address it eventually.

@SteveSanderson
Copy link
Contributor

Thanks everyone (especially @mbest) for investigating and fixing the legacy browser issues! I've looked into it a bit further and found there were just a few remaining issues relating to HTML parsing. Please see the pull request #1866 for details.

Once this stuff is sorted, I think the code is ready to ship. Obviously we still need to do the other things in the checklist at the top of this page (docs, etc.).

Regarding #1360 (migrate to Gulp) - how close are we to merging that? Brian, you mentioned that the debug output is nearly byte-for-byte identical. Could you elaborate on whether the remaining differences might be consequential in any way? Since #1360 now uses Closure Compiler, I'm guessing that once the debug build is equivalent to Grunt's output, the release build will be equivalent to Grunt's too. Is that right? If so, I'd love to get this merged in ASAP. Ideally before 3.4.0 ships, but if not, then let's at least commit to it being the absolute next thing that goes in, and hold off anything else going to master until it does. Agreed?

@rniemeyer
Copy link
Member

@SteveSanderson - I would agree that #1360 should be the next thing.

@brianmhunt
Copy link
Member

@SteveSanderson The build from #1360 compares to master as follows

After running both, in directories for their respective branches,

  • master/ $ grunt
  • 1360/ $ gulp build:closure

The following command illustrates the differences in the debug:

root@ko-tmp:~# diff master/build/output/knockout-latest.debug.js
  1360/build/output/knockout-latest.debug.js
6,8d5
<
< (function(){
< var DEBUG=true;
11a9
>     /* const */var DEBUG = true;
48c46
< ko.version = "3.3.0";
---
> ko.version = "3.3.0-debug";
5836,5837c5834
< }());
< })();
---
> }());

The only semantic difference here is that the DEBUG has been moved inside the wrapper function, and the debug version now has the -debug suffix (which I believe is semver-ok and probably useful since for the moment there are substantive differences between the compiled version and -debug variant; I am of course happy to back this change out though).

The compiled versions are identical, except for one extra newline in 1360 that I have not found yet.

A note on releasing: While there is a release task, and it closely follows some related discussion on releasing, I have not tested this thoroughly or recently, so while it may work please take it only as a guideline for now or test it out before using.

There will undoubtedly be other considerations, advancements, and refinements we can make to the build process, but there is nothing I can think of that should hold up merging #1360.

Incidentally, the above is set up in a VM that I can share, if you send along a public ssh key.

@brianmhunt
Copy link
Member

Incidentally, it may be worthwhile to rebase to clean up the commit log for #1360.

@mbest
Copy link
Member Author

mbest commented Sep 4, 2015

I'd prefer not to merge #1360 right now and hold up the release. Instead let's look at merging it right after. Also I'd prefer to get the benefits without it being spread through 139 commits. Ideally, there should be just enough commits to aid understanding of the changes.

@mbest
Copy link
Member Author

mbest commented Sep 5, 2015

I've come up with a possible fix for #1622 (attached to the issue). @SteveSanderson, you've put a lot of thought and work recently into the ko.computed code, so it'd be good to get your feedback on it.

@mbest
Copy link
Member Author

mbest commented Sep 10, 2015

I've been thinking that after releasing the new version, the Knockout code should not be left with the actual 3.4.0 version string, but possibly something like 3.5.0-development.

@rniemeyer
Copy link
Member

should not be left with the actual 3.4.0 version string, but possibly something like 3.5.0-development.

@mbest - makes sense to me

@mbest
Copy link
Member Author

mbest commented Sep 10, 2015

Are we planning to add Edge to our list of supported browsers?

@brianmhunt
Copy link
Member

@mbest If going down the road of a stable version and a development version, it probably sense to maintain a release branch and master branch in parallel (or, conversely, a dev and master).

The development branch would get the -dev suffix, and the release branch would keep the released version (in this case 3.4.0).

This might sound a bit like git-flow, but I think that git-flow would probably needlessly restrictive.

I think it makes sense to support Edge.

@IanYates
Copy link

Great work!!! 💯

I'm keenly looking forward to the docs around deferred updates and to hear experiences from others. I know @mbest has articles on his blog from a while ago. I've just downloaded the RC and if I enable deferred updates, some parts of my site where I'd gone to great effort to batch updates myself now break (understandably - it's an RC and I'm using two 'deferred' bits in conflict with each other). In effect I get some very tight loops so IE gives "long running script" messages and Opera/Chrome eventually crash...

I'd rather move to baked-in deferred updates instead of my custom batching but I need to better understand the corner cases, etc so I can gradually migrate. I'm happy to share my experiences (and perhaps finally start a blog!) but will wait for the docs before digging in too deep.

My use case: I get some data coming in via SignalR which needs to update the screen. I can get a lot of small objects in succession, all of which end up in an array and possibly having many other parts of the screen waiting on their arrival. I'd rather wait a little bit and let quite a few through in one hit than update the array many times. In IE it can make the difference between a 10 second pause and a 10ms pause. Throttling, rate-limit, etc aren't quite appropriate since it's not just array-based work I'm doing and the observable chain is, for programming convenience and to be nicely reactive to server data changes, not always a simple A->B->C relationship.

@mbest
Copy link
Member Author

mbest commented Oct 13, 2015

Thanks for the feedback, @IanYates. It's possible that you have recursive logic in your code that depends on synchronous behavior. A common example is using some sort of "updating" flag that prevents code from running in response to certain changes:

var isUpdating = false;
observable1.subscribe(function(value) {
    if (!isUpdating) {
        isUpdating = true;
        observable2(somecomputedvalue);
        isUpdating = false;
    }
});
observable2.subscribe(function(value) {
    if (!isUpdating) {
        isUpdating = true;
        observable1(somecomputedvalue);
        isUpdating = false;
    }
});

The deferred code will stop running and throw an error if it detects a high level of recursion, but perhaps the recursion in your application isn't detected by Knockout (or you didn't wait long enough).

@melck
Copy link

melck commented Oct 14, 2015

I'm really excited by this release and i want to migrate some large applications who need performance improvement..

For now, i just tried 3.4.0rc but i see multiple recursion errors like this :

Uncaught RangeError: Maximum call stack size exceeded

If i understood well, all computed that have a synchronous behaviour, need to be changed to prevent recursion. What is the best way to migrate them ? Do i need to add flags everywhere ?

Thank you for the work done 👍 and sorry for my poor english

@mbest
Copy link
Member Author

mbest commented Oct 14, 2015

@melck I'm guessing you're seeing that because of #1905.

@melck
Copy link

melck commented Oct 15, 2015

@mbest Yes indeed. But if i want to use deferred updates or get performance boost, how can i fix this problem ? Could you give me a simple example of how to properly handle computed recursion ?

@aligneddev
Copy link

FYI: We ran our Selenium UI and Jasmine tests (which we have good coverage of our code and UI) and they all passed. I don't have any numbers on performance improvements (we don't have a good bench marking system in place), but we are excitedly awaiting the full release so we can upgrade.

Thanks for all the good work!

@Eisenspalter
Copy link

I update to version 3.4 and now I have a different render behavior. In version 3.3 the container was shown immediately before all the bindings in the container were finished. In version 3.4 the container is not visible until all bindings are finished. So sometimes the user wait 2-3 second and nothing happens. I use components for my bindings.
My container component has an with and a visible binding
.container(data-bind="with:binding, cssVisible:visible") I think that could be the reason for the different behavior. The shorter response time in version 3.3 gives the user a better experience. What can I do to get the old behavior?
Since I use components massively everything seems to be slower. Are components slower than templates or native html? Is there percentage how slower it is? Perhaps guys, you can help me.

@mbest
Copy link
Member Author

mbest commented Oct 17, 2015

@Eisenspalter That's interesting. With 3.3.0, the components each render in a separate setTimeout callback, so overall the initialization would take longer. How long does it take for everything to initialize with 3.3.0? You can try one thing: add ko.tasks.scheduler = function (callback) { setTimeout(callback, 0); }; and see what it does.

@Eisenspalter
Copy link

@mbest It's is not easy to say how long it take, because I don't know when the binding is finished. The important thing for me and the user experience is, that 3.3 does not block. The user can interrupt each the process. Knockout 3.4 blocks. That is not a good idea in Javascript. The second benefit of non blocking binding is, that you can do parallel things, like css3 animation for showing and hiding containers. That is a very nice user experience and reduce the waiting time for the users.

@Eisenspalter
Copy link

@mbest ko.tasks.scheduler = function (callback) { setTimeout(callback, 0); }; I tried this in 3.4. It looks like that only the first binding use the scheduler function. All following bindings does not use it.

@jrsearles
Copy link

The 3.4 pre-release is not showing up in Bower. I'm guessing that it'd because the release is tagged as 3.4.0rc and Bower might be expecting 3.4.0-beta.

@melck
Copy link

melck commented Oct 19, 2015

@jrsearles it works like that:

bower install knockout#v3.4.0rc

@jrsearles
Copy link

Right, which is what i've done. But it's not easily available in the current tag. (ie, bower info knockout does not display this version, whereas prior alpha/beta releases are available.)

@mbest
Copy link
Member Author

mbest commented Oct 20, 2015

I've changed the tag name to v3.4.0-rc (added the hyphen). So now it shows up in Bower.

@melck
Copy link

melck commented Oct 21, 2015

@mbest the tag doesn't have dist folder. So it does't work with bower

@mbest
Copy link
Member Author

mbest commented Oct 21, 2015

Good catch @melck. I've fixed it now.

@mbest
Copy link
Member Author

mbest commented Oct 25, 2015

It's been about two weeks since the RC release, and we've fixed a couple of problems that were discovered: #1903 and #1905.

Other reports:

  1. Unresponsiveness when using components. We could mention this in the release notes, but there doesn't seem to be a workaround.
  2. "Too much recursion" error using deferred updates (that doesn't happen when using 3.3.0 with the plugin). The native deferred updates version isn't the same as the plugin. Perhaps we should mention this in the release notes along with a link to migration docs.
  3. Error when passing computed or observable methods as a callback. We should mention this in the notes.

I think we should wait a little bit to see if we get any more feedback on the first two issues to be sure we're describing the situation clearly.

@brianmhunt
Copy link
Member

Thanks @mbest for keeping 3.4 moving forward! In light of the issues raised, I agree on waiting a little bit.

@mbest
Copy link
Member Author

mbest commented Nov 1, 2015

Draft release notes for final 3.4.0 release (edit as needed).

New features and bug fixes

  • Improves performance of components, templates, computeds, and observables.
  • Includes a native version of deferred updates, along with a microtask queue (ko.tasks).
  • Calls a ko.onError handler, if defined, for errors from asynchronous code.
  • ko.options.useOnlyNativeEvents can be set to tell Knockout to use only native (not jQuery) events.
  • Includes ko.isPureComputed().

The 3.4.0 RC release notes has the full list of issues and pull requests included in this release. The final release fixes two regression bugs found in the RC:

Possible compatibility issues

  1. Components now use microtasks to perform updates asynchronously instead of setTimeout. Since microtasks are run before the browser repaints the page, all loaded components will be initialized and displayed in a single repaint. Although this reduces the overall time needed to display components, it could result in a longer delay before anything is displayed.
  2. The new, native deferred updates feature has a slightly different API and is implemented differently than the Deferred Updates plugin. Migrating from the plugin will generally require some code changes (full details to come soon).
  3. ko.observable and ko.computed no longer use a closure when defining their methods, such as dispose and valueHasMutated. These functions expect this to be set correctly and so can't be used directly as a callback. Instead you'll need to use bind, such as obs.dispose.bind(obs).

@michaelaird
Copy link

How about putting out an RC2 nuget package?

@mbest
Copy link
Member Author

mbest commented Nov 17, 2015

I've released 3.4.0 now. @SteveSanderson can you update NuGet?

@mbest mbest closed this as completed Nov 17, 2015
@aligneddev
Copy link

Is there anything that needs to be updated for the d.ts in DefinitelyTyped/DefinitelyTyped#4288?

@brianmhunt
Copy link
Member

Awesome, thanks @mbest

@jonbnewman
Copy link

Thanks for the hard work guys!

@bludev
Copy link

bludev commented Nov 20, 2015

Hi guys! What's new about NuGet release?

@SteveSanderson
Copy link
Contributor

Fantastic - great to see this release out! Brilliant work, especially @mbest for pushing through the actual release process.

@bludev I'll put out the NuGet release early next week - hope that's OK. I'm away at the moment and not in reach of a machine set up to do that.

@PaulHuizer
Copy link

Thanks for this new release. @mbest +1 !!!

@SteveSanderson
Copy link
Contributor

The 3.4.0 package is now published to NuGet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests