search

pyrsmk / toast

A modern JS/CSS asset loader, written in TypeScript.
MIT License
118 stars 13 forks source link

Missing Timeout Argument? #2

Closed ghost closed 12 years ago

ghost commented 12 years ago

I don't know if it's just a forgotten argument or what, but Line 34 is just setTimeout(isComplete); with no timeout in miliseconds. Is this correct? If not, I just read over the MDN doc on setTimeout and it might be helpful to note that 4ms is the minimum but 10 is safer cross-browser.

Edit: Line 43 and 62 are missing it as well.

pyrsmk commented 12 years ago

I didn't forgot the timeout argument, I simply dropped it to save bytes because my tests show me that the timeout parameter was not necessary.

But, if it's safer, maybe we should put a 10ms timeout for all setTimeout functions...

pyrsmk commented 12 years ago

Fixed with bfc4416d83c72b7d7e0d65aedef8815cd3c43248

ghost commented 12 years ago

Yeah, the second argument is always safest. I tried setTimeout without a time argument on Node.js and it doesn't work.

Anyway, I'm really loving the simplicity and size of Toast. Great work.

pyrsmk commented 12 years ago

Thanks, I always try to keep KISS in mind when developing ;)