wilsonpage / fastdom

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

Wrapper functions .reader() .writer() #63

Closed wilsonpage closed 8 years ago

wilsonpage commented 9 years ago
var writeSafe = fastdom.writer(function() {
  el.innerHTML = 'new stuff';
});

var readSafe = fastdom.reader(function() {
  return el.clientWidth;
});

// With Promise
writeSafe().then(() => ...);
readSafe().then(result => ...);

// Without Promise
writeSafe(() => ...);
readSafe(result => ...);
wilsonpage commented 9 years ago

@paullewis would this be useful?

paullewis commented 9 years ago

I think so, yep. I think it should ideally do the warning if you read during a write and vice-versa. What do you think?

wilsonpage commented 9 years ago

Would this require an internal registry of all props and methods?

On Sat, 25 Jul 2015 08:33 Paul Lewis notifications@github.com wrote:

I think so, yep. I think it should ideally do the warning if you read during a write and vice-versa. What do you think?

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

paullewis commented 9 years ago

Yes, it would. But we have that, at least on the Blink side. I'm assuming Gecko is similar?

aFarkas commented 9 years ago

I made some additions to implement a reader/measurer. Here is one with the most simple implementation and here one with a promise or promise-like API.

But I have two questions about your discussion:

  1. writer/reader without promise, but with callback as first argument I have not implemented this, because I think it is wrong. We want to be able to pass arguments to the writer or reader that are used by the actual read/write task.

Here is an example:

var writer = fastdom.writer(function(elem, width){
    elem.style.width = width +'px';
});

As soon as we allow a callback function to be passed things got messy. We have a promise and for browsers which don't we have something much more basic with a thenable object.

I think the promise based thing is quite easy to work with. Also note, that the promise is resolved with the return value of the reader/writer:

$.fn.readWidth = fastdom.reader($.fn.width);
$.fn.writeAddClass = fastdom.reader($.fn.addClass);

$('.box').readWidth().then(function(width){
    $('.box').writeAddClass(width > 600 ? 'large-component' : 'medium-component');
});

Is this good or did you have something else in mind.

  1. Implement warnings

    it should ideally do the warning if you read during a write and vice-versa.

How should this be implemented? I don't understand.

wilsonpage commented 9 years ago

Implement warnings > it should ideally do the warning if you read during a write and vice-versa.

We could either check function.toString() for known read APIs or spy on known read apis (SinonJS style) for the duration of the user's function call.

Both would require a list of known getters/functions bundled within fastdom.js. I suggest it could be an optional plugin.

aFarkas commented 9 years ago

Ok, I understand. I don't think this is achievable.

Both things create performance problems.

  1. function.toString() doesn't work: If you use a lib or helper methods to read/write things you don't see any reads or writes because they are wrapped in another function.

In my own fastdom implementation, that has a writer implementation, I do this the hole time.

  1. Proxying DOM things won't work in Safari.

Maybe it is achievable as kind of a not trustable debugger plugin. That can be only used in most modern browser and should never be included in production.

Are you fine with the promise-like only approach?

wilsonpage commented 9 years ago

How will it fallback for browsers without Promise?

aFarkas commented 9 years ago

It returns an object with a then method, that can consume callback functions: https://github.com/aFarkas/fastdom/blob/v1-beta-writer-promise/lib/fastdom.js#L366-L384

Of course if someone uses this feature extensively and wants reliable results, he/she should use a Promise polyfill.

wilsonpage commented 8 years ago

If this is still useful it can be implemented as an 'extension'. The warning use-case is now covered by fastdom-strict.