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

What is this, java? #37

Closed davidhellsing closed 10 years ago

davidhellsing commented 10 years ago

Why all the namespace code-magic? it makes it nearly impossible to port this otherwise great tool to newer systems like browserify or webpack. You should really consider dropping some of the actionscript/java legacy and move towards a CommonJS compatible script without messing around with window this and com.greensock.core that. Not only is it really difficult to follow, it also takes up a lot of valuable coding space.

I’d like to do a simple require('TweenLite') but... how? I’ve been hacking around with the source but it’s a dependency mess right now. No offense.

It shouldn’t be a great deal for you that knows the inner workings of f.ex https://github.com/greensock/GreenSock-JS/blob/master/src/uncompressed/TweenLite.js#L74-L95

Maybe just don’t do your own dependency management, instead use requireJS or something else for development and then wrap it up in nice little packages for distribution...?

Let me know if I can help.

JamesLAllen commented 10 years ago

I've been using TweenLite with requirejs since the early GSAP JS beta versions. Simply define your AMD module like you would another library like jQuery. GSAP plugins are smart enough to not initialize until either TweenLite or TweenMax has been loaded (or TimelineLite or TimelineMax too). There is no need to define any custom shims either, unlike other libraries used with AMD

Here's an example for your paths:

'TweenMax': '../vendor/greensock/src/minified/TweenMax.min',
'greensock/plugins/CSSPlugin': '../vendor/greensock/src/minified/plugins/CSSPlugin.min',

And I call them as dependencies like so:

define(['TweenLite','greensock/plugins/CSSPlugin'], function(){ 
     // ... don't execute until TweenLite and CSSPlugin are available  ...
 });

I also use Bower however bower's system is still lacking a little for libraries with multiple files, so I do need to manually update paths for each external library, however this isn't exclusive to TweenLite. Hope this helps!

davidhellsing commented 10 years ago

Hey @Mode7James, thanks, but I’m not talking about requireJS, I’m talking about Common JS, like npm, browserify, webpack etc.

And when I look at the code for internal dependency handling and namespacing, I kind of get the feeling that this is a legacy solution that could benefit from improvement towards supporting newer systems.

jackdoyle commented 10 years ago

Dependency management was definitely a challenging issue that we put a lot of thought into when we created the JS flavor. We didn't want to use RequireJS for a lot of reasons (file size, 3rd party dependency, only one class definition allowed per file, packaging headaches, etc.), so we rolled our own that allows things to be loaded in any order, accommodates any number of class definitions in a file, and uses less kb. Sorry if the code was a little tough to decipher.

Keep in mind that the goal with that system had nothing to do with legacy code, although for convenience we did use the namespacing that was industry standard in Flash (com.greensock.core, for example) but that wasn't costly at all in terms of file size or anything else. We just tacked it on for consistency. I'm sure some of the Flash folks appreciated that, and it also gave us a way to organize things in an object tree.

Confession: I'm not at all familiar with browserify or webpack. I'm also not intimately familiar with CommonJS.

I did a lot of research during the original port and found absolutely nothing out there that adequately addressed my concerns (perhaps I just missed something). If you've got specific ideas for how to improve things, I'm all ears.

Oh, and are you aware of the GreenSockGlobals that you can [optionally] define? Maybe this'll help: http://forums.greensock.com/topic/9494-node-webkit-support/#entry38490

davidhellsing commented 10 years ago

Thanks for the answer @jackdoyle! You know, it’s your project so you may do dependency management as you please. The thing is that you are not alone out there with javascript libraries handling dependencies, and you might benefit from using standardized methods like commonJS modules instead of rolling your own if you want to reach a wider audience.

F.ex, you are assuming that there is a window global and you push things in a queue into the global in your definitions. That is not the way you want to do things in a module environment - each module has it’s own requirements that should be met before exporting the module.

Maybe I’m missing something, but you do have duplicate code in your scripts, are you manually copying them or are you concatenating these "modules" when building your scripts? In that case, you might be closer to a module structure than you might think.

Consider this pseudo code for f.ex TweenMax:

var EasePack = require('easeing/EasePack');
var TimelineMax = require('TimelineMax')

module.exports = function() {
    // EasePack, TimelineMax etc... are loaded and avail
    // Do the TweenMax magic
}

Then we can just do this in our requireJS / webpack / browserify environment:

var TweenMax = require('TweenMax')

No namespace, no globals, no duplicate code, just a really simple pattern to follow :) If you want to pack things up so the usual crowd gets a single script tag, just use browserify or requireJS in your dev environment (no extra kb) and pack it up using nodeJS before pushing it to git.

Have a look at how f.ex jQuery does it in their github repo, they use requireJS or something similar in their dev environment to modularize different features of their code, then they pack it up for distribution. But you can also clone the entire repo and use nodeJS to load a single module if you want.

Another thing: the "com.namespace.foo" was industry standard in java (f.ex (http://docs.oracle.com/javase/tutorial/java/package/namingpkgs.html) and AS3 adapted it when it was moving closer to java. And no-one, I mean nobody in the JavaScript world uses that convention.

Sorry for being brutally honest about this, I just think your excellent script deserves a wider audience of developer environments besides the usual jQuery crowd (I used your AS3 versions back in the day!)

davidhellsing commented 10 years ago

Added a pull request to show you an example on proper module exporting based on your current setup. This has been tested in Browserify, RequireJS and window/browser environments: #38

But I still think you should consider moving dependencies into a modulerized system.

matthewwithanm commented 10 years ago

The big benefit with these module systems is that the modules can both be consumed individually (for people using browserify, webpack, etc) and used to generate builds for people who want to use it "the old fashioned way."

For example, if you use CommonJS, people using browserify or webpack could require('greensock/TweenLite'). You could use browserify to generate a standalone build from the same code which would expose globals for those who just want to put something in a script tag.

matthewwithanm commented 10 years ago

@jackdoyle This code is particularly problematic and I'm not exactly sure what it's meant to do. module is going to refer to the TweenLite module, which means that every time a new Definition is created (for example by CSSPlugin), it's going to replace TweenLite (and any subsequent requires will get the module that corresponds to the last created Definition object).

tombigel commented 10 years ago

btw, you have this project - https://github.com/umdjs/umd That has all the relevant module patterns templates

heltonvalentini commented 10 years ago

+1 for this.

jerryorta-dev commented 10 years ago

@jackdoyle, as a previous Flash developer that used your AS3 framework extensively, I appreciate the efforts you have taken to make the transition easy. But now I'm deep into angular.js, node.js, and have worked with require.js. So I think we ( actually you ) have enough time and history from the flash days to let those concepts retire and embrace javascript modular development entirely, especially in a future state.

You have a lot to think about to be truly future state of modular development. For example: Angular 2.0 is being built for ES6 ( google created their own compiler to work ahead ), which means other companies/frameworks will follow suite soon when ES6 is officially released. See http://blog.angularjs.org/2014/03/angular-20.html

Point being ES6 brings the "import" and "export" statements, as well as the class architecture with public variables, getters and setters that look very similar to java and flash. So I don't think you are far off with your current implementation in a future state, but need to work on getting out of the global space per previous posts. See http://addyosmani.com/writing-modular-js/, Look for "Modules With Imports And Exports", and "Classes With Constructors, Getters & Setters".

ianmcgregor commented 10 years ago

Some documentation for using TweenMax within different module systems would be a good addition.

Using Browserify (Common JS module style) I had to dig around in the source to figure out that I needed to do:

require('./path-to/TweenMax.js');

And then use as TweenMax.to (as in window.TweenMax.to)

The 'to' method is undefined if I do the usual:

var TweenMax = require('./path-to/TweenMax.js')

matthewwithanm commented 10 years ago

@ianmcgregor I'm not sure about how browserify treats broken modules, but I think that may not work if you try to use a plugin (for the reason I mentioned above). I know that for webpack I had to create a custom loader that basically tricks the library into not using its (broken) module.exports assignment.

ianmcgregor commented 10 years ago

@matthewwithanm Thanks, yes you're right. I can't find a way to bundle TimeLineMax either...

matthewwithanm commented 10 years ago

Yeah, you're only going to be able to use one thing since Definition overrides TweenLite's (or TweenMax's) exported value every time it's used.

I know what I'm doing in webpack can be done in browserify but I don't know the specifics OTTOMH. The basic idea, though, is to have the build tool set the module object to null and provide a dummy window so that the bad code never runs. Then use a shim to get what should be exported.

@jackdoyle I know the OP was pretty combative but this is a pretty serious issue that makes using GSAP with a modern JS build system really hard. Just to discover that it really is a GS bug, the user has to really grok its custom module system, how modules work in node, and the inner workings of their build tool. Can you chime in about what kinds of PRs you would accept? In the long term, GS definitely needs to move to UMD but in the interim, even just removing the broken node export would probably be an improvement.

jackdoyle commented 10 years ago

Thanks for the input, guys. I need to do some more research on this. One of the challenges has been that things like RequireJS only allow one class to be defined per file, but that's a no-go for GSAP since it's important for our users not to have to wrestle with that many files. I'd be happy to look at any specific recommendations that would accommodate the needs of build systems as well as packaging things neatly and conveniently for users who aren't at all comfortable (or interested in using) build systems, and wouldn't break legacy projects where people are just loading a TweenMax.min.js file (for example).

Again, thanks for the input and we are listening - just need to explore some things as soon as my schedule frees up a bit. We've been working on some features and other tweaks.

matthewwithanm commented 10 years ago

@jackdoyle Awesome. As far as the "multiple files" thing goes, that's not actually the case. Each of the tools mentioned here has a way to generate a single-file standalone build for exactly the reasons you give: Browserify has the "standalone" option, RequireJS has an optimizer called r.js, and webpack has library and externals. My personal recommendation would probably be to use Browserify. I don't think that the actual bundling will be an issue at all; the more complicated thing will be figuring out how to resolve the duplication in the Lite and Max versions. Are you currently just duplicating that by hand or is there some unseen build step behind the files in src/?

jackdoyle commented 10 years ago

Curious: why do you recommend Browserify? What do you believe are the strengths/weaknesses of the various options? (others please feel free to chime in here with your own opinions...)

matthewwithanm commented 10 years ago

Webpack is the most powerful, but somewhat difficult to configure. I use it for projects but I think it's overkill for just generating a standalone build. Between browserify and r.js, I think browserify has more momentum behind it. Many (most?) people prefer the CommonJS style modules to the AMD wrapping. I also appreciate its modular design—there's a whole ecosystem of (first and) third party transforms.

ianmcgregor commented 10 years ago

In my experience:

Gulp+Browserify builds are much faster than Grunt+RequireJS builds, which is important for a smooth workflow.

The CommonJS module style (Browserify) is cleaner and simpler than AMD (Require).

It feels like Browserify gives you a bit more flexibility over Require (using browser-compatible node modules directly, more control over build process).

Browserify makes it easy to create 'standalone' bundles (i.e. a library which consists of a bunch of JS modules packed into one file with a public API). This can be done with Require by using a lib called Almond.

Set-up seems to be a bit more complicated with Gulp+Browserify than Grunt+Require, but once you get there you're good!

Libraries like PIXI.js seem to handle things quite well without using any module system, by simply concatenating all the files and using a UMD like this:

(function(){
    var root = this; // window

    var PIXI = PIXI || {};

    // all the PIXI library code here

    if (typeof exports !== 'undefined') {
        if (typeof module !== 'undefined' && module.exports) {
            exports = module.exports = PIXI;
        }
        exports.PIXI = PIXI;
    } else if (typeof define !== 'undefined' && define.amd) {
        define(PIXI);
    } else {
        root.PIXI = PIXI;
    }
}).call(this);
heltonvalentini commented 10 years ago

I also think you should usue Browserify since it allows you to easily write your code using CommonJS module patterns and you get the whole npm ecosystem to support your development. Also if you write your code in CommonJS it'll be a lot easier to convert ot ES6 modules when the time comes. Comparison between CommonJS and ES6 modules If you wanna know more about Browserify: Designing Modules for the Browser and Node with Browserify Browserify Handbook

jackdoyle commented 10 years ago

Thanks for the input, guys. It seems Browserify has some solid momentum. I'll have to look into what the file size tradeoffs are too if we restructure things. Does anyone have tips on how to prevent bloat in the final concatenated/minified files like TweenMax.min.js when using all the require() stuff and segregating each and every class (Animation, Ease, EventDispatcher, TweenLite, TimelineLite, etc.)?

matthewwithanm commented 10 years ago

If you use CommonJS (module.exports) and the standalone option, the bloat should be minimal—especially when gzipped.

JonDum commented 10 years ago

Just throwing this out there, but Webpack is similar to Browserify but offers some additional features that may tailor to GSAP's use case.

Specifically, Webpack was designed for Code Splitting which would allow the 'lightweight' & 'robust' bundles that are in use today while still allowing the code to be written in either a CommonJS or AMD format (Browserify only supports CommonJS format) for those who would like create their own bundles with only what they need.

davidhellsing commented 10 years ago

Just chipping in my 2 cents; sorry for being slightly combative in the original post @jackdoyle , but at least I’m glad this issue got your (and others) attention :+1:

I would definetely check out browserify first. Browserify+gulp is really easy to get going with because it’s all JavaScript functions and the standalone option in Browserify is really convenient to get the UMD exports without writing repetitive code. I have tried WebPack because my favourite framework React uses it, but I was getting more control over the build process in Browserify/Gulp setup.

File size (adding require etc) should not be an issue when moving to CommonJS, minifying an gzipping bundles will generally produce equal if not better user experiences when it comes to loading scripts.

If you need help with setting up gulp+browserify for development & deployment I would gladly assist as I have done a lot of build scripts for this in the past.

tombigel commented 10 years ago

I don't want to spoil the Browserify party here, but here at Wix we decided eventually not to use Browserify and build our own lean packaging mechanism with Grunt and RequireJS.

The main concerns were that Browserify, while simplifying the process, adds constrains that when applied on an existing project with an existing package logic creates some problems, mainly in development and debugging environment from day one:

  1. With Browserify default packaging mechanism the Development code and the running code are essentially different, which makes debugging harder, and forces you to use maps etc. to pinpoint problems in the code.
  2. Without Browserify it is much simpler to inject packages in realtime while debugging
  3. The development overhead of using "native" RequireJs and building a simple package that conforms to our existing packages logic was much smaller than converting the exisiting code to work in the "Browserify way".

Just some point to think about.

davidhellsing commented 10 years ago

@tombigel some valid points there. I sort of agree on 1. but I think sourcemaps are good enough for debugging. And the fact that you can test/run the site with compiled and deployable code is actually an advantage. I have never had the need for injecting packages while debugging so I can’t say anything about that.

I had a lot of resistance towards Browserify in the beginning, because I never like the idea of involving server-side when developing. So I also started out with requireJS, but very soon I realized that commonJS is the way to, mainly because of the server/client code-sharing possibilities, the rich eco-system and the standardized syntax from NodeJS. We also had some troubles getting requireJS to compile properly when using complex module structures.

And after working with modules in general there is no turning back.

tombigel commented 10 years ago

I now also experience problems with the fact that GSAP is dependent on the global scope, and cannot be required as a whole package.

@jackdoyle do you have any estimate if or/and when you are going to implement requireJS/CommonJs support?

jackdoyle commented 10 years ago

Can't commit to a timeframe yet (LOTS on my plate and it seems like rewriting everything CommonJS-style will take a while), but I'm curious: are you aware of the GreenSockGlobals thing? Is it not a viable solution for you in the meantime? This may or may not be a helpful thread to read through as well: http://forums.greensock.com/topic/7961-trouble-with-requirejs-rjs-and-tweenmax/#entry30491

davidhellsing commented 10 years ago

@tombigel I ported the current TweenMax and TweenLite into proper module exports here: https://github.com/greensock/GreenSock-JS/pull/38 and it works in my tests (including the browser). I didn’t change much, as you can see here: https://github.com/greensock/GreenSock-JS/pull/38/files

Maybe that could help?

@jackdoyle I think everyone understands your limited plate, and we are all grateful for your contributions. But I think that a proper export of the most popular tween scripts would be a really good first step, and it doesn’t require much code to be altered (from what I can see). The GreenSocksGlobal is not much help, because you are still defining it in the window scope.

jackdoyle commented 10 years ago

David, I'd like to run something past you offline that serves as the first step you're talking about - can you shoot me your email address so I can get you access?

davidhellsing commented 10 years ago

@jackdoyle yes of course, add me at david at aino dot se :+1:

jackdoyle commented 10 years ago

Thanks again for your help making 1.13.1 much more compatible with things like RequireJS, Browserify, etc. We'll continue to explore ways to modularize things further down the road, but in the mean time, 1.13.1 should work just fine in these environments. Let me know if that's not the case.