fregante commented 8 years ago

The current setup works great in most situations but it's not entirely npm/node/modules friendly. Let me show some of the limitations of the current setup, which may not be big on their own, but they pile up.

Ease of import (✅ fixed in 1.19.1)

Importing a plugin or just a part of GSAP is cumbersome

import TweenLite from 'gsap/src/uncompressed/TweenLite';
import Draggable from 'gsap/src/uncompressed/utils/Draggable';

By just moving/copying the js files to the root of the npm package (not on GitHub) one could import them this way:

import TweenLite from 'gsap/TweenLite';
import Draggable from 'gsap/Draggable';

Chance of code duplication

I can import TweenLite and TweenMax in three ways and in larger projects this could easily happen:

// main.js
import 'part1.js';
import 'part2.js';
import 'part3.js';

// part1.js
import {TweenLite} from 'gsap';

// part2.js
import TweenLite from 'gsap/src/uncompressed/TweenLite';

// part3.js
import TimelineMax from 'gsap/src/uncompressed/TimelineMax';

The resulting main.js will have the entire TweenLite.js, TimelineMax.js, and TweenMax.js (which include both of the previous ones already).

This likely works thanks to GSAP' own plugin system, but the output will be heavier to download, parse and execute.

See #180

Automated third-party optimization

Because of its proprietary bundling/plugin system, GSAP users completely miss out on optimizations like tree-shaking, which are becoming more and more common.

A ESModules-enabled GSAP would allow the ease of this:

import {TweenLite, Draggable} from 'gsap';

While keeping the lightness of the carefully-selected:

import TweenLite from 'gsap/src/uncompressed/TweenLite';
import Draggable from 'gsap/src/uncompressed/utils/Draggable';

Without requiring tooling-specific configurations (https://github.com/greensock/GreenSock-JS/issues/165, https://github.com/greensock/GreenSock-JS/issues/157#issuecomment-229055039)

Notes

All of this would not come at the expense of everybody else since you'd still offer pre-packaged ready-to-use modules.

While backwards compatibility for npm users could be maintained, semantic versioning (i.e. publishing 2.0.0) would not break existing installations even if you remove the src folder from the published package.

Bonus points: having an npm-specific build will make the code even lighter because you don't need to include UMD.

Related: Three.js just now accepted a PR by @Rich-Harris to refactor the code into proper ES Modules.

jackdoyle commented 8 years ago

Thanks for the detailed suggestions (and summary of benefits), Federico. You've always been a great friend to GSAP over the years. We're certainly moving in the direction of a completely rewrite in ES6 but that's no small task. It's going to take some time to complete. Thanks for your patience.

glebmachine commented 8 years ago

Waiting with bated breath

max-ch9i commented 7 years ago

@jackdoyle Are there any updates on this?

jackdoyle commented 7 years ago

@maximus8891 Nothing to share yet. We've been hard at work on several other features :) but you should totally be able to use GSAP in a build system - you just might need to add some aliases in your config file, that's all. It's mentioned in several other posts here. Like:

resolve: {
  root: path.resolve(__dirname),
  extensions: ['', '.js'],
  alias: {
    "TweenLite": "src/libs/gsap/TweenLite",
    "CSSPlugin": "src/libs/gsap/plugins/CSSPlugin"
  }
}

If anyone else wants to offer an index.js file that'd just serve as an exporting wrapper, I'd be happy to look at it.

max-ch9i commented 7 years ago

Thanks, @jackdoyle. That's how I have been doing it so far.

fregante commented 7 years ago

If anyone else wants to offer an index.js file that'd just serve as an exporting wrapper, I'd be happy to look at it.

I just saw this. I wasn't able to make to enable tree-shaking without making your files proper ES modules (e.g. export default TweenLite in TweenLite.js)

This was my test code:

webpack --entry ./src.js --output-filename dist.js --module-bind 'js=imports-loader?define=>false'

gsap/everything.js: (With a non-tree-shaking bundler you get all the files, so it's best not to save it as index.js)

import TimelineLite from './src/uncompressed/TimelineLite';
import TimelineMax from './src/uncompressed/TimelineMax';
import TweenLite from './src/uncompressed/TweenLite';
import TweenMax from './src/uncompressed/TweenMax';
import EasePack from './src/uncompressed/easing/EasePack';
import jQueryGsap from './src/uncompressed/jquery.gsap';
import AttrPlugin from './src/uncompressed/plugins/AttrPlugin';
import BezierPlugin from './src/uncompressed/plugins/BezierPlugin';
import CSSPlugin from './src/uncompressed/plugins/CSSPlugin';
import CSSRulePlugin from './src/uncompressed/plugins/CSSRulePlugin';
import ColorPropsPlugin from './src/uncompressed/plugins/ColorPropsPlugin';
import DirectionalRotationPlugin from './src/uncompressed/plugins/DirectionalRotationPlugin';
import EaselPlugin from './src/uncompressed/plugins/EaselPlugin';
import EndArrayPlugin from './src/uncompressed/plugins/EndArrayPlugin';
import ModifiersPlugin from './src/uncompressed/plugins/ModifiersPlugin';
import RaphaelPlugin from './src/uncompressed/plugins/RaphaelPlugin';
import RoundPropsPlugin from './src/uncompressed/plugins/RoundPropsPlugin';
import ScrollToPlugin from './src/uncompressed/plugins/ScrollToPlugin';
import TextPlugin from './src/uncompressed/plugins/TextPlugin';
import Draggable from './src/uncompressed/utils/Draggable';

export {
    TimelineLite,
    TimelineMax,
    TweenLite,
    TweenMax,
    EasePack,
    jQueryGsap,
    AttrPlugin,
    BezierPlugin,
    CSSPlugin,
    CSSRulePlugin,
    ColorPropsPlugin,
    DirectionalRotationPlugin,
    EaselPlugin,
    EndArrayPlugin,
    ModifiersPlugin,
    RaphaelPlugin,
    RoundPropsPlugin,
    ScrollToPlugin,
    TextPlugin,
    Draggable,
};

src.js

import {TweenLite, Draggable} from 'gsap/everything';

jackdoyle commented 7 years ago

Thanks @bfred-it. You mentioned "With a non-tree-shaking bundler..." - are you saying that tree-shaking isn't really possible with this or were you simply warning folks that they'd benefit from having tree-shaking enabled if they don't want everything loaded? And that's why you don't recommend making this index.js, right?

fregante commented 7 years ago

I'm saying it depends on the user's bundler. If you use browserify + babel on my example you'll get a 900KB file. So it's not advisable as an index.js file but rather as an opt-in — at least until tree shaking doesn't become more widespread and automatic.

Currently though tree shaking doesn't work at all because the existing files aren't ES modules, I think.

jshrssll commented 7 years ago

@jackdoyle how do you suggest importing TweenMax when the javascript is rendered on the server? Getting the age-old "document is not defined" error.

jackdoyle commented 7 years ago

@splurtcake Would you mind trying the beta [uncompressed] TweenMax file at https://s3-us-west-2.amazonaws.com/s.cdpn.io/16327/TweenMax-latest-beta.js and letting me know if that's working better for you?

Also, do you know how to quickly reproduce the issue you were running into without digging into how to create a server and compiling there? I'm not familiar with that process and I have a few deadlines I'm working against, so I don't have time to dive into it at this point. I'm looking for a quick way to reproduce what you were seeing, hopefully just using the browser or a node command.

jshrssll commented 7 years ago

@jackdoyle thanks for getting back to me so quickly. I'll take the beta for a spin now and see how it goes.

Unfortunately there is no quick way to reproduce as you need a Node server + angular 2 universal running. It was a nightmare to set up in the first place haha. I'll let you know how I go with the beta 👍

fregante commented 7 years ago

You might have to load jsdom when doing the server-side rendering. I'm not sure how else you can convince all library authors to include a document check. 😕

This, however, is not related to modules. Perhaps open a new issue.

jshrssll commented 7 years ago

@jackdoyle almost there... Now I'm just getting the following error

ERROR calling setTween() due to missing Plugin 'animation.gsap'. Please make sure to include plugins/animation.gsap.js

I'm importing animation.gsap with this import 'imports-loader?define=>false!scrollmagic/scrollmagic/uncompressed/plugins/animation.gsap';

jshrssll commented 7 years ago

Why the thumbs down @bfred-it ?

fregante commented 7 years ago

This is off-topic. Open a new issue please.

jackdoyle commented 7 years ago

That looks like a ScrollMagic thing, not GSAP. We didn't author ScrollMagic, so we can't do much about that :( Maybe reach out to its author.

fregante commented 7 years ago

Going back to topic: perhaps it's early to have a ES module entry point. It worked for Three.js because it used to be a all-in-one package and the change didn't affect existing users; while gsap's situation is core (in more than one flavor) + plugins. So there goes point 3

Also when I opened this issue I hadn't clear in mind how the TweenMax and TweenLite objects were related: from what I understand now, TweenMax does not include+extend TweenLite but it fakes its variable — Correct?

If so: there's no solution to the duplicate code that comes out of require('gsap/TweenLite'); require('gsap/TweenMax'); other than a message somewhere that says TweenLite is already included in TweenMax; you don't need to require it

So there goes point 2 (although https://github.com/greensock/GreenSock-JS/issues/180#issuecomment-269645321 still applies)

So now it comes down to ease of import, which should be easy to implement. This is what the ideal npm package content should look like:

./README.md
./package.json
./LICENSE

./TimelineLite.js
./TimelineMax.js
./TweenLite.js
./TweenMax.js
./EasePack.js
./jquery.gsap.js
./AttrPlugin.js
./BezierPlugin.js
./CSSPlugin.js
./CSSRulePlugin.js
./ColorPropsPlugin.js
./DirectionalRotationPlugin.js
./EaselPlugin.js
./EndArrayPlugin.js
./ModifiersPlugin.js
./RaphaelPlugin.js
./RoundPropsPlugin.js
./ScrollToPlugin.js
./TextPlugin.js
./Draggable.js

./TimelineLite.min.js
./TimelineMax.min.js
./TweenLite.min.js
./TweenMax.min.js
./EasePack.min.js
./jquery.gsap.min.js
./AttrPlugin.min.js
./BezierPlugin.min.js
./CSSPlugin.min.js
./CSSRulePlugin.min.js
./ColorPropsPlugin.min.js
./DirectionalRotationPlugin.min.js
./EaselPlugin.min.js
./EndArrayPlugin.min.js
./ModifiersPlugin.min.js
./RaphaelPlugin.min.js
./RoundPropsPlugin.min.js
./ScrollToPlugin.min.js
./TextPlugin.min.js
./Draggable.min.js

All files are at root level (lodash v4 made the same change. See "More modular" section) so this means that requires are as easy as:

const TweenLite = require('gsap/TweenLite');
const Draggable = require('gsap/Draggable.min'); // minified version 4 characters away!
// or
import TweenLite from 'gsap/TweenLite';
import Draggable from 'gsap/Draggable.min';

This change can be made by either:

copying the files to the root on postinstall (non-breaking)
creates change of duplication because gsap/TweenLite and gsap/uncompressed/TweenLite are separate files
copying the files to the root on prepublish (non-breaking)
same as above, plus brings npm package size to literally 666KB
symlinking the files to the root on postinstall (non-breaking) I'm not familiar with OS/bundler compat for this, maybe they still see links as separate files
moving the files to the root (breaking, needs v2) no issues ✅

I think solution 2 is the easiest to have now and then when v2 comes drop the minified/uncompressed folders.

jackdoyle commented 7 years ago

TweenMax does not fake a TweenLite variable. It literally has the TweenLite code inside of the TweenMax file (the goal was to make it simple for folks to just load one file and get all the essentials plus some extras - simplicity). TweenMax indeed extends TweenLite.

As for the "ease of import" suggestion, I have a few questions:

Are you suggesting making a DUPLICATE of every plugin and ease and utility and drop them into the root directory alongside TweenLite/Max, etc.?
If so, that'd also show up in the github repot, thus probably look pretty messy, right? I'm not saying it's not worth that tradeoff - I just want to make sure I'm understanding you correctly and weighing the tradeoffs.
Minified files too? What's the benefit there? My understanding is that almost everyone who is consuming GSAP via a build system is likely going to have a minification step built in, and it's a better practice to use the uncompressed files so that troubleshooting is easier. Right? I just want to avoid unnecessary copying of all these resources and cluttering up of the directories if I can.
You previously suggested creating that "gsap/everything.js" file; are you saying that you prefer this "copy everything into the root" strategy instead?

Thanks for all the suggestions and details, @bfred-it !

fregante commented 7 years ago

Yes; duplicate them until you're ready to break compatibility in v2
No, everything described here would only apply to what you're distributing on npm.
I personally never require minified files but a) they're already in your npm package and b) some people might want them (as shown in #180). I'm fine without them.
Correct. That gsap/everything.js was the entry point for modules/tree-shaking bundlers, but it feels like it's not worth it at the moment (I was referring to this with "there goes point 3")

TweenMax does not fake a TweenLite variable

Does that mean that, with little work, you can literally leave require('./TweenLite') and require('./CSSPlugin') inside TweenMax.js? (leaving aside current compatibility/usage issues)

That's great news then! It might be good to have as a second step (or perhaps in v2)

fregante commented 7 years ago

I assume you have some sort of build step in place, can you shed light on it? How do you include TweenLite in TweenMax currently?

jackdoyle commented 7 years ago

Thanks again for all your help on this, Federico.

1.19.1 is out and the published (NPM) package has all of the uncompressed files flat in the root to make importing easier, so you should be able to do any of these:

//typical import for things inside TweenMax:
import {TweenLite, Power2, TimelineMax, TweenMax} from "gsap";

//or to get to things that aren't in TweenMax...
import Draggable from "gsap/Draggable";
import ScrollToPlugin from "gsap/ScrollToPlugin";

Sn0wFox commented 7 years ago

Beware, for those using both NPM and Bower with Webpack 2 (it could happen in projects with old dependencies...), if webpack is configured to use the package.json before the bower.json, i.e.:

resolve: {
    // ...
    descriptionFiles: ['package.json', 'bower.json'] 
}

and that gsap was installed with bower, then none of the following imports work:

import { TweenLite }  from 'gsap';  // Nope
import TweenMax from 'gsap';        // Nope
import TweenMax from 'gsap/src/uncompressed/TweenMax';  // Nope

To solve it, we must tell webpack to look in the bower.json file first, i.e.:

resolve: {
    // ...
    descriptionFiles: ['bower.json', 'package.json'] 
}

fregante commented 7 years ago

* or don't use bower at all because it just causes issues like the above.

Sn0wFox commented 7 years ago

@bfred-it Totally agree, but sometimes you land on an old project and you don't have a choice. True story 🍷

greensock / GSAP

Improvements for build systems (Modules) #170

Ease of import (✅ fixed in 1.19.1)

Chance of code duplication

Automated third-party optimization

Notes