greensock / GSAP

GSAP (GreenSock Animation Platform), a JavaScript animation library for the modern web
https://gsap.com
19.56k stars 1.72k forks source link

Draggable requires a global navigator instead of _gsScope.navigator #212

Closed robballou closed 7 years ago

robballou commented 7 years ago

When loading the libraries in a node-based test, Draggable will fail to load because it can't find the navigator object. I'm not actually trying to test anything related to Draggable, it's just imported in code that is tested.

In Draggable:

_isAndroid = (navigator.userAgent.toLowerCase().indexOf("android") !== -1), //Android handles touch events in an odd way and it's virtually impossible to "feature test" so we resort to UA sniffing

In TweenMax, somewhat similar:

_agent = (_gsScope.navigator || {}).userAgent || "",
jackdoyle commented 7 years ago

Ah, fair point. Thanks for the suggestion. Here's a preview of the next release - better?: https://s3-us-west-2.amazonaws.com/s.cdpn.io/16327/Draggable-latest-beta.js (uncompressed)

robballou commented 7 years ago

That works for _agent, but then I encounter an error with

/Users/example/git/project/node_modules/gsap/Draggable.js:387 _SVGElement = window.SVGElement,

I'm using browser-env to set globals for document, window, navigator, but I think the closure doesn't pick these up (just guessing). Thank you for taking a look at this. Currently I can get around it by just not importing it in my code and including the JS via a script tag in the project.

jackdoyle commented 7 years ago

Ah, okay. How about now? https://s3-us-west-2.amazonaws.com/s.cdpn.io/16327/Draggable-latest-beta.js

robballou commented 7 years ago

Haha. Still running into issues with that. I'm willing to concede that including this just won't work or I can make a simple project that can be used for testing it if that helps.

Thank you for trying to help out :)

jackdoyle commented 7 years ago

Yeah, a simple test project would be great. What problem did you run into with the latest one? Same line? Got a suggested workaround?

fregante commented 7 years ago

The problem is that with server prerendering Greensock will be run in node. Either you detect this (typeof window === 'undefined') once and exit, or you end up having to add the check on every mention of browser globals.

To find the errors, just try to run the file from the command line:

node Draggable.js
ernestbofill commented 6 years ago

Hi, I just came across the same problem importing Draggable. My project is written in TypeScript and has Server Side Rendering (so it runs in Node). I had to import those scripts globally using <script> tags, but it's definitely not ideal. Would be great if all files could be imported in node.

jackdoyle commented 6 years ago

Is there something specific you think can be done in Draggable to resolve this? I tried @bfred-it's suggestion of running node Draggable.js but that doesn't work at all (dependencies). And it seems a bit odd to expect something like Draggable to work outside the browser (server-side only) since it's interaction-driven and dependent on many things in the browser. But again, if there's a specific recommendation for a tweak that can be made to Draggable, I'm totally open to that.

ernestbofill commented 6 years ago

Yes Draggable can be adjusted to resolve this. In fact most of GreenSock libraries and plugins seem to be compatible already. By the way, changes should go to the umd version because Node doesn't support ES6 modules at this point.

"And it seems a bit odd to expect something like Draggable to work outside the browser (server-side only) since it's interaction-driven and dependent on many things in the browser"

I doesn't have to "work" outside the browser, it just needs to be importable so that it doesn't break isomorphic applications

"running node Draggable.js but that doesn't work at all (dependencies)."

Depending files should also be there. Both TweenLite and CSSPlugin are already Node compatible so they can be imported without problems.

I think the solution would be trying to replicate what TweenLite is doing. Or something like what they suggest in this post

ernestbofill commented 6 years ago

@jackdoyle, let me know if you need more info. I can probably help with this, it should be quite simple: changing 4-5 lines of code would do the trick.

jackdoyle commented 6 years ago

@ernestbofill Sure, I'm busy with a few other things at the moment but I'd love to get your specific recommendations. Thanks!

jackdoyle commented 6 years ago

@ernestbofill - just checking to make sure you got my response. I'd love to get your input on this in order to make sure we're on the same page.

ernestbofill commented 6 years ago

@jackdoyle I think a good solution would be to always access window, navigator or document from _gsScope, but I don't know how _gsScope works, so you probably know more than me on that. Another case to take care of is when creating elements dynamically or manipulating elements as that will not be possible when running in Node.js

Do you accept PRs? I can try to make a PR if I find the time. Probably in a few weeks though.

jackdoyle commented 6 years ago

We don't typically accept PRs, but it'd definitely be useful for you to submit one just so I can see the exact changes you're recommending. If/When you're ready to help with some testing, let me know and I can send you revised files.

jackdoyle commented 6 years ago

@ernestbofill Would you mind trying these files and reporting back if you notice any problems? https://greensock.com/temp/Draggable.zip

ernestbofill commented 6 years ago

@jackdoyle I just tried the files that you sent me. The esm file is not relevant because it uses syntax that is not supported in Node.js

The other files I got it to work with a few adjustments. This is what I did:

  1. Copied the file to the folder where other umd gsap files are so that it can find the dependencies. node_modules/gsap/umd in case of an npm project.
  2. Run node Draggable.js from the command line in that folder (I use Node 8).
  3. I get this error:
    
    C:\Projects\HorizonCopy\src\Horizon.Ng.Library\node_modules\gsap\TweenLite.js:21
    export const _gsScope = (typeof(window) !== "undefined") ? window : (typeof(module) !== "undefined" && module.exports && typeof(global) !== "undefined") ? global : this || {};
    ^^^^^^

SyntaxError: Unexpected token export

It is because the require paths point to non-umd versions of `TweenLite` and `CSSPlugin`. So I fixed it by changing lines 2489 and 2490 to this:
```ts
  require("./TweenLite.js");
  require("./CSSPlugin.js");
  1. Next try I get this other error:
    
    C:\Projects\HorizonCopy\src\Horizon.Ng.Library\node_modules\gsap\umd\Draggable.js:925
                                div.appendChild(child);
                                    ^

TypeError: div.appendChild is not a function at C:\Projects\HorizonCopy\src\Horizon.Ng.Library\node_modules\gsap\umd\Draggable.js:925:9



This is because the dummy element defined in line 27 doesn't provide this function. So I change line 27 to this: `_dummyElement = {style:{}, appendChild: () => {}, removeChild: () => {}},`.

1. Now I just get a message `GSAP encountered missing dependency: plugins.CSSPlugin` but I think it's okay because I didn't add that plugin. Nevertheless I'm a bit confused because line 2490 is requiring this file.

1. But basically with these changes in place it seems to run.
jackdoyle commented 6 years ago

Thanks for the feedback, @ernestbofill. I hope to have this in the next release (pretty soon).

jackdoyle commented 6 years ago

Released 2.0.2 👍