timstuyckens / chromeextensions-knockoutjs

The source code for the chrome dev tools extension that allows you to easily debug knockout js apps.
MIT License
140 stars 50 forks source link

Constantly hangs the browser #9

Closed slaneyrw closed 11 years ago

slaneyrw commented 11 years ago

I would love to use this extension but I find it constantly hangs the developer tools window.

Anything I can do to work why this occurs ?

timstuyckens commented 11 years ago

Does it happen with any website ? For example, knockoutjs.com ? Do you use a custom templating engine? (Maybe related to https://github.com/timstuyckens/chromeextensions-knockoutjs/issues/8 )

If you could provide a small page that can reproduce the problem I will take a look.

slaneyrw commented 11 years ago

It is on a website we're developing inhouse, and yes we do use a custom template engine ( koExternal )

On 7 June 2013 16:33, timstuyckens notifications@github.com wrote:

Does it happen with any website ? For example, knockoutjs.com ? Do you use a custom templating engine? (Maybe related to #8https://github.com/timstuyckens/chromeextensions-knockoutjs/issues/8)

If you could provide a small page that can reproduce the problem I will take a look.

— Reply to this email directly or view it on GitHubhttps://github.com/timstuyckens/chromeextensions-knockoutjs/issues/9#issuecomment-19091241 .

timstuyckens commented 11 years ago

In the weekend I'll try to reproduce the issue with the examples in https://github.com/ifandelse/Knockout.js-External-Template-Engine

timstuyckens commented 11 years ago

Hi,

I tried to reproduce the issue with the examples but no luck. @brianmhunt I think you have the same problem.

If you guys have any idea's on how I can reproduce it, please let me know

amd_native jquery nativ2

acchu1986 commented 11 years ago

Hi @timstuyckens,

I am encountering the same issue with the knockout external template engine. The browser hangs constantly or the knockout context doesn't display anything. I am using RequireJS also..

Thanks

timstuyckens commented 11 years ago

Any public available site on which I can reproduce the issue? Is you setup different from the examples in the examples in https://github.com/ifandelse/Knockout.js-External-Template-Engine ?

timstuyckens commented 11 years ago

I suspect that it is the ko.toJS method one the context or the data. Could you try executing

  ko.toJS(ko.dataFor($0))    

and

ko.toJS(ko.contextFor($0).$root)

in the page that causes the problem (after selecting a DOM node in the dev tools)?

Issue #10 had some dom (not serializable to json) elements in the view.

7 had a wrong inheritance pattern that caused crashes.

Maybe this is simular ...

I just did a release (2.3) with additional error handling.

JamesMessinger commented 11 years ago

I'm experiencing the same hanging issue, and it's definitely related to the JSON serialization. The ko.toJS method leaves a lot to be desired when it comes to JSON serialization. I recommend that you write your own serialization function instead. Some of the things that the ko.toJS method doesn't support include:

1) Serializing DOM elements I know you've previously argued that there is no reason to have DOM elements in binding contexts, but this is actually a fairly common pattern when using some JavaScript libraries. For example, Backbone views have an el property and a $el property, both of which represent DOM elements. It should be fairly easy to add logic to skip DOM elements during JSON serialization.

2) Circular references For example, an element might be bound to an Employee object, which has a company property that is a Company object. The Company object has an employees property that is an array of Employee objects, including the Employee object that the element is bound to. Viola! Circular reference. JSON serialization will go into an infinite loop unless logic is added to detect objects that have already been serialized and not serialize them again.

3) Custom toJSON methods According to the JSON.stringify spec, objects can define their own toJSON method to customize their JSON serialization. For example, Backbone models and collections define their own toJSON methods, but ko.toJS ignores them and attempts to do its own serialization (which fails).

timstuyckens commented 11 years ago

@BigstickCarpet

Short: Later today I'll publish a new version that makes the use of ko.toJS optional (checkbox in the options page). It is still on by default because it provides the best experience. ko.utils.unwrapObservable will be used as fallback.

Long: I don't agree with your arguments (1) and for (2,3) I don't know why you said them (at least with the current version of knockout), but I admit that hanging should not happen

1) DOM elements in VM I don't see the link between a Backbone view and a MVVM viewmodel. Knockoutjs is the glue between code and markup and you should not mix these. If you need to manipulate the DOM in some way, use a custom binding. Custom bindings are a really nice way of solving those problems (plugins, animations, ...).

2) Circular references javascript ko.toJS supports circular references. See the unit tests in https://github.com/knockout/knockout/blob/master/spec/mappingHelperBehaviors.js#L56 I added youre example locally and run the tests and they succeeded:

    it('ko.toJS should resolve circular references', function() {
        var Employee=function(name,theCompanyTheEmployeeWorksFor){
            this.name=name;
            this.company=theCompanyTheEmployeeWorksFor;
        };
        var Company=function(){
            var self=this;
            self.hasProfit=true;
            self.employees=[
                new Employee("e1",self),
                new Employee("e2",self)
            ];  
            self.bestEmployee=ko.observable(new Employee("be",self));
        };
        var comp=new Company();
        var result = ko.toJS(comp);
        expect(result.bestEmployee.name).toEqual("be");
    });

3) Custom toJSON methods Again, this works... There already was a test in the mappingHelperBehaviors for arrays and locally I added the following passing test for objects:

    it('ko.toJS use a custom toJSON function when available', function() {
    var data = {
            a: "test1",
            b: ko.observable("test2")
        };
        data.toJSON = function() { return "custom toJSON" };
        var result = ko.toJSON(data);

        // Check via parsing so the specs are independent of browser-specific JSON string formatting
        expect(typeof result).toEqual('string');
        var parsedResult = ko.utils.parseJson(result);
        expect(parsedResult).toEqual("custom toJSON");
    });

So I don't see why I should spend time writing a custom javascript serialization library

timstuyckens commented 11 years ago

2.5 is live. If you disable serialization, I hope you don't have this isssue any more.

optionscontextdebugger

knockoutjs context debugger options - google chrome

JamesMessinger commented 11 years ago

Thanks! The 2.5 update works great! With the "Serialize" option enabled, the browser still hangs... but only for a couple seconds, then the data shows up in the KnockoutJS panel. But with the "Serialize" option disabled, there's no hang at all, and the data shows up immediately.

Thanks for adding this option! This extension is tremendously helpful and invaluable to debugging complex Knockout apps. Keep up the great work!

brianmhunt commented 11 years ago

No hangs for me in 2.5 either; so far so good! Many thanks @timstuyckens :beers:

timstuyckens commented 11 years ago

Alright, thanks for the feedback !

SeriousM commented 10 years ago

If you disable serialization, I hope you don't have this isssue any more.

wow, thank you so much! now I can continue working with this extension :)