wilsonpage / fastdom

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

use module pattern instead of singleton #20

Closed Rich-Harris closed 11 years ago

Rich-Harris commented 11 years ago

This is quite a ballsy pull request, feel free to disregard entirely if you see fit. Basically I wanted to satisfy my own curiosity, and thought I may as well share the result.

I was wondering what the rationale was for using a singleton rather than a module? As a module, all the internal guts (e.g. the unique ID, _add and so on) are hidden, and you don't have to create a new raf(function() { self._frame(); }) for each request when pending === false, you can just do raf(frame).

It's about 20% smaller minified (because there are no unmangleable this or property names), though gzipped the difference is negligible (a testament to gzip's efficiency).

Anyway as I say I won't be offended if you ignore this PR! Cracking work on the library, I think this will change the way a lot of people approach DOM manipulation.

wilsonpage commented 11 years ago

Even though fastdom is exposed as a singleton, I like the fact that it's an instance of FastDom. This makes things easier for testing, as I can instantiate a new FastDom instance for each test (accessible via fastdom.constructor). This means tests don't share state, and therefore don't conflict when run in parallel. Thanks for the work, but for those reasons I'm not going to merge :(

My hope is that if my app uses FastDom and I have a dependency that also uses FastDom we never have more than one instance in action. Perhaps tests need to be written to prove this works :/

Rich-Harris commented 11 years ago

No worries, appreciate the explanation