-
Notifications
You must be signed in to change notification settings - Fork 27
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
Create polyfills for supporting older browsers #4
Comments
Om uses (if (exists? js/requestAnimationFrame)
(js/requestAnimationFrame #(render-all state))
(js/setTimeout #(render-all state) 16)) so I think it fits most cases. (def request-animation-frame
(if (exists? js/window.requestAnimationFrame)
js/window.requestAnimationFrame
#(js/window.setTimeout % 16))) |
(if (.-addEventListener elem)
(.addEventListener elem (name actual-type) canonical-f)
;; For IE < 9
(.attachEvent elem (name actual-type) canonical-f)))) (if (.-removeEventListener elem)
(.removeEventListener elem (name actual-type) canonical-f)
;; For IE < 9
(.detachEvent elem (name actual-type) canonical-f)))) |
Sounds good. Would you want to make a pull request so you'll get credit? ;) |
We could also try to capture vendor prefixes and make sure there's a performance timestamp (although I'm not really sure this part is necessary): https://gist.github.com/timhall/4078614 |
I think attachEvent might need the "on" prefix: http://stackoverflow.com/questions/2657182/correct-usage-of-addeventlistener-attachevent. I also intend to allow people to optionally plug-in goog.events or another handler system if they want a well tested event subsystem. For instance one could require |
Yes, I will make pull request after we come to agree of preferred solution. What about size increase on using google events — it is to be tested, may be it will be negligible. |
I think this solution is overengineering with doubtful value. We must not rely on any particular fps — so we don't need precise timing. Also, if our render cycle is too heavy then it seems that logic is quite heavy too and we want to give it 16ms to proceed, not to shrink its time. And, if our render cycle is heavy, we should remember that setTimeout polyfill does not stop rendering when tab is inactive (requestAnimationFrame does), so we don't want to make high cpu load when inactive, so empirical 16ms pause is also good in this case
Yes, but now I think that we have no necessity to support attachEvent. IE8 has too many problems aside of this one to drop its support, and IE9+ has addEventListener, so we have no problems with it. |
So I agree with what you're saying about precise timing. We could just try to pull in the vendor prefixes and if not, use the setTimeout with 16ms like Om. |
#17 polyfill arrived ;-) |
Okay, sounds good. I think that polyfill looks pretty good so I'm going to go ahead and merge it. |
I did ul@cybercraft ~/S/f/bench> du *
176K goog-frk.min.js
172K native-frk.min.js goog.events adds about 4K, which is about 2.3%. I think it is not much, especially keeping in mind that it is minimal example, real app will be bigger. Also, we can make it pluggable. What is the best way to make this? Allow to pass |
Cool. How much is that gzipped? We can just have a separate namespace that calls set! on the listen! var - On Sunday, November 23, 2014, Ruslan Prokopchuk notifications@github.com
|
ul@cybercraft ~/S/f/bench> du *
48K goog-frk.min.js.gz
44K native-frk.min.js.gz delta is the same =) |
Is that your whole project? On Sunday, November 23, 2014, Ruslan Prokopchuk notifications@github.com
|
No, it is example from freqctive README. Give 5 minutes, I will post benchmark using my project codebase. |
Hmmm... I'm already using ul@cybercraft ~/M/m/bench> du -b *
474370 goog.js
474373 native.js
ul@cybercraft ~/M/m/bench> gzip *
ul@cybercraft ~/M/m/bench> du -b *
121756 goog.js.gz
121757 native.js.gz |
So, I looks like freactive by itself isn't that heavyweight right now. On Sunday, November 23, 2014, Ruslan Prokopchuk <notifications@github.com
|
created PR #21 as |
Core.async uses goog.async.nextTick: https://github.com/google/closure-library/blob/master/closure/goog/async/nexttick.js See the comments in goog.async. Works down to IE6. HTH |
https://github.com/google/closure-library/blob/master/closure/goog/async/nexttick.js#L15
|
Doh. Ignore this then. I was too quick with the post. Sorry for the noise. |
No problem, you are welcome! |
Let me try this again. This should be it: https://github.com/google/closure-library/blob/master/closure/goog/async/animationdelay.js |
Yes, looks like it is the one, thank you. @aaronc, what do you think? |
Good polyfills for things like
requestAnimationFrame
,addEventListener
, etc. to support older browsers where feasibleThe text was updated successfully, but these errors were encountered: