drawcall / Proton

Javascript particle animation library
https://drawcall.github.io/Proton/
MIT License
2.41k stars 275 forks source link

requestAnimationFrame polyfill should either be removed or safe for server #63

Closed cha0s closed 4 years ago

cha0s commented 4 years ago

I want to be able to run Proton on a server, where window doesn't exist. You can still simulate particles even if you don't actually render them.

This is currently not possible, as Proton does import './polyfill/requestAnimationFrame';, which throws something like:

.../node_modules/proton-js/src/polyfill/requestAnimationFrame.js:9
        for (var x = 0; x < vendors.length && !window.requestAnimationFrame; ++x) {
                                         ^
ReferenceError: window is not defined
    at window (.../node_modules/proton-js/src/polyfill/requestAnimationFrame.js:9:42)
    at .../node_modules/proton-js/src/polyfill/requestAnimationFrame.js:6:3
    at .../node_modules/proton-js/build/proton.min.js:10:84
    at Object.<anonymous> (.../node_modules/proton-js/build/proton.min.js:10:149)
    at Module._compile (internal/modules/cjs/loader.js:701:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:712:10)
    at Module.load (internal/modules/cjs/loader.js:600:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:539:12)
    at Function.Module._load (internal/modules/cjs/loader.js:531:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)

on a server environment.

I don't see that requestAnimationFrame is actually used anywhere in the engine. Only the examples.

cha0s commented 4 years ago

I recommend total removal as I don't think that a global polyfill for raf is Proton's responsibility.

drawcall commented 4 years ago

Very good advice, can I look at the code you submitted tomorrow? Sorry, it’s already late.

cha0s commented 4 years ago

Take your time and have a good night

drawcall commented 4 years ago

Very good change, I have merged into the master branch (can you help verify all the examples?). I also removed it in proton 4.0.

cha0s commented 4 years ago

I did check the examples locally when I was developing the pull request, everything seemed to be working fine!