wilsonpage / fastdom

Eliminates layout thrashing by batching DOM measurement and mutation tasks
6.83k stars 240 forks source link

would a synchronous mode for debug be a good idea? #41

Closed trullock closed 9 years ago

trullock commented 10 years ago

I'm finding that when debugging through my code, it can sometimes be very tricky to know what state things are in, especially when chasing rendering bugs.

Would a switch on fastdom to put it into a synchronous mode be useful? Just for debug, obviously.

so I could do fastdom.async = false; or similar, which would make read() and write() execute instantly instead of using rAF etc.

okonet commented 10 years ago

I think it also would be useful for tests.

matthew-andrews commented 10 years ago

Chrome now supports asynchronous debugging:- http://www.html5rocks.com/en/tutorials/developertools/async-call-stack/

Also I think test frameworks let you have asynchronous tests - the one I have most familiarity with (buster) will let you have async tests by returning a promise at the end of the test...

So I think the answer to this question might possibly be no.

wilsonpage commented 10 years ago

@matthew-andrews is wise. I trust him :)

trullock commented 10 years ago

Yeah, Fastdom's error handling and browser debug tools have advanced sufficiently to make this redundant

cobbweb commented 10 years ago

I'm executing fastdom.runBatch() in my tests which runs anything scheduled, which makes everything synchronous without issue.

Is there a simple way to clear anything scheduled in fastdom (read: disregard anything scheduled without executing)?

wilsonpage commented 10 years ago

@cobbweb nice tip! Perhaps we could implement a fastdom.clearAll() type API, is that what you need?

wilsonpage commented 10 years ago

Or perhaps just calling fastdom.clear() with no arguments could clear all jobs.

wilsonpage commented 10 years ago

I would also like a better error catching solution if anyone has any ideas.

felixlaumon commented 10 years ago

I think fastdom.clear() should remain as a noop if there is no args or when the id is undefined or null to prevent clearing all jobs accidentally. For example,

var jobId = fastdom.write(...);
// In somewhere else ...
jobId = null;
fastdom.clear(jobId);
wilsonpage commented 10 years ago

I was thinking we'd check arguments.length to avoid that problem

felixlaumon commented 10 years ago

Yeh that will work too

pyramids commented 10 years ago

Whilst FastDom.prototype.runBatch at https://github.com/wilsonpage/fastdom/blob/master/index.js#L240 is documented as "private", my first impression is that it should solve the given problem. And if used only for debugging, maybe it is acceptable that it is not technically meant to be called from outside fastdom itself? Is there a good reason not to expose it as public API?

EDIT: Deleted lots of irrelevant words about error handling arising from temporary misunderstanding of current code.

wilsonpage commented 10 years ago

Let's discuss an ideal error catching API...

pyramids commented 10 years ago

@wilsonpage If that is a reply to me using such words, apologies, as I only wrote those due to not initially realizing the current implementation essentially already does what I wanted to suggest (throw or rethrow exceptions not handled with a fastdom.onError handler, such that generic asynchronous exception handling solutions apply). I already edited that initial post to try and avoid an unnecessary discussion.

wilsonpage commented 10 years ago

@pyramids, do you not think the error handling needs improving then?

pyramids commented 10 years ago

@wilsonpage No, as best as I can tell right now, it actually already does everything much as I had wanted to request. Again, apologies if my initial post was prematurely suggesting otherwise; my mistake.

wilsonpage commented 10 years ago

No sweat, I'm just looking for ways to make things better :)

On Tue, Aug 12, 2014 at 7:52 PM, Björn Stein notifications@github.com wrote:

@wilsonpage https://github.com/wilsonpage No, as best as I can tell right now, it actually already does everything much as I had wanted to request. Again, apologies if my initial post was prematurely suggesting otherwise; my mistake.

— Reply to this email directly or view it on GitHub https://github.com/wilsonpage/fastdom/issues/41#issuecomment-51959796.