Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Having problems with CommonJS implementation #232

Closed
arscan opened this issue Aug 12, 2014 · 23 comments
Closed

Having problems with CommonJS implementation #232

arscan opened this issue Aug 12, 2014 · 23 comments

Comments

@arscan
Copy link

arscan commented Aug 12, 2014

I'm trying to use velocity using browserify in the simplest case (without externally loading velocity and using browserify shim), and I get the error

Velocity: Either jQuery or Velocity's jQuery shim must first be loaded.

It looks like the code won't execute require("jquery") unless window.Velocity is defined, which doesn't make sense:

https://github.com/julianshapiro/velocity/blob/master/jquery.velocity.js#L42

I suspect the point of that conditional is to allow you to use Velocity's jQuery shim instead of jQuery. If that is the case, shouldn't it be:

factory(window.Velocity ? undefined : require("jquery"));

That seems to be consistent with the AMD implementation as well as the downstream code. It also gets rid of the error for me.

@julianshapiro
Copy link
Owner

Fixed. See new release.

@arscan
Copy link
Author

arscan commented Aug 12, 2014

Works great. Thanks Julian 👍

I'm not doing any shimming and it seems to work fine w/browserify now. My code is super simple (taken from the svg animation example):

var $ = require("jquery"),
    velocity = require("velocity-animate");

$(function(){
    $("#rect").delay(500)
    .velocity({ x: "+=202", y: "25%" })
});

@haustraliaer
Copy link

I had to remove ? undefined : require("jquery") - then it worked with the shim.

Otherwise it would still look for the jquery module in my browserify bundle.

@julianshapiro
Copy link
Owner

Essentially, here's what's going to happen:

The shim -- about 4kb of code extracted from jQuery -- will be inlined into a new Velocity version that will go by velocity.js (instead of jquery.velocity.js). The shim code will literally just be pasted above all the existing Velocity code -- even before all the module loading code we're referring to.

The shim code registers a dummy window.Velocity var, which that module loading code block looks for to see if the shim exists. If the shim exists, jquery doesn't get required. If the shim code doesn't exist (if window.Velocity isn't defined), then jquery gets required as a dependency.

@julianshapiro
Copy link
Owner

Please update to the latest version and let me know how everything is working.

@haustraliaer
Copy link

I still have the same problem with browserify - it will try and find the jquery module even if the shim is present. Just to clarify, this is how I'm requiring the code:

I have velocity.js:

var shim = require('./shim.js');

/* the latest jquery.velocity.js code */

And application.js:

var Velocity = require('./velocity.js')

Velocity.animate(myElem, etc, etc)

When I try to build the bundle with browserify I get the error module "jquery" not found.... If I remove the if/else statement it all works fine. So my factory code looks like this:

    /* CommonJS module. */
    if (typeof module === "object" && typeof module.exports === "object") {
        module.exports = factory(window.Velocity);
    /* AMD module. */
    } else if... etc.

@julianshapiro
Copy link
Owner

Window never had anything to do with it :)

I have a main.js with:

var Velocity = require("jquery.velocity.js");

Velocity.animate(document.body, { opacity: 0.5 }, 2000);

Where jquery.velocity.js has the shim's code inlined at the very top of the file. Works perfectly for me. Maybe you're not using the latest version of the shim and there's an error there. I just emailed you my current copy.

@haustraliaer
Copy link

I've got it to work fine - it just needs to not call the jquery module (which prevents browserify from building if it's not present) - after that the shim works for me.

@fattenap
Copy link

I have a feeling that I need a shim.js file. Is this correct? Where would I find this file?

Thanks

@julianshapiro
Copy link
Owner

@fattenap: Yeah, you need a special shim file. I had emailed it to David a long time ago.

Leave me your email and I'll send it

Thanks for writing all that up. Sorry about the confusion.

@haustraliaer No rush, but looking forward to your response -- close to releasing the shim publicly.

@fattenap
Copy link

@haustraliaer

Have you tried to run browserify with the -r flag. eg. browserify ./main.js -r ./velocity.jquery-shim.js:jquery > bundle.js

This pulls the shim into your bundle and assigns jquery to it.

This works for me.

@haustraliaer
Copy link

Allright I got it all sorted - after trying @fattenap's method of using the -r flag (which worked), I managed to also replicate it in gulp using the shim option:

gulp.task('scripts', function(){
  gulp.src(['./app/application.js'])
    .pipe(browserify({
      shim: {
        jquery: {
          path: './app/lib/velocity/shim.js',
          exports: 'jquery'
        }
      }
    }))
    .pipe(gulp.dest('./build/assets/js/'));
});

So now I'm able to npm install velocity-animate without modifying anything (so long as I have that shim file) and it all works as expected. Not sure what the implications will be for inlining it though as we couldn't use the -r flag or the gulp shim method if the file itself wasn't present.

Also, @haustraliaer, your homepage is amazing.

Ahaha which one the pokemon thing? Just something I was messing about with a while ago - it's due for an update, this time I think with some sweet velocity animations ;)

@julianshapiro
Copy link
Owner

Sorry, one last question: Are you guys sure that simply pre-concatenating the shim and jquery.velocity.js into one file and just loading that one file doesn't work? Because it should work in all environments. And, if it doesn't, I need to figure out why it works for me but not for you guys.

@fattenap
Copy link

I can confirm that it works as expected by pre-concatenating the shim to the top of jquery.velocity.js and running browserify without the require flag. :)

But it still pulls in jQuery so you will need to run browserify with the -u flag

browserify ./main.js -u jquery > bundle.js

This ensures jQuery is not included with your bundle.

@haustraliaer you may want to try something like this.

gulp.task('scripts', function() {
   return browserify('./app.js')
      .exclude('jquery')
      .bundle()
      .pipe(source('bundle.js'))
      .pipe(gulp.dest('app')));
});

Thanks

@fattenap
Copy link

Hi @julianshapiro

Do you think that the line should be

module.exports = factory(window.Velocity ? undefined : window.jQuery || require("jquery"));

That way if I exclude jQuery from my bundle it can still be referenced via a script tag

@julianshapiro
Copy link
Owner

Sure, why not. Expect it to be changed in the next update.

@julianshapiro
Copy link
Owner

;)

@fattenap
Copy link

Sorry,

Doing that breaks it. :(

Keep getting

Velocity UI Pack: Velocity must be loaded first. Aborting. if jQuery is specified in a script tag.

@fattenap
Copy link

It actually needs to be this

module.exports = factory(window.Velocity ? window.jQuery : require("jquery"));

this works

@Prinzhorn
Copy link

No rush, but looking forward to your response -- close to releasing the shim publicly.

Did you release the shim? Why is it even needed?

I'm currently simply doing require('velocity-animate'); and was hoping it would add itself as a jQuery plugin. But instead I end up with a global window.Velocity and $element.velocity is undefined. I'm using browserify and expect no globals to be added whatsoever.

@julianshapiro
Copy link
Owner

hey @Prinzhorn, is the module loading overview here helpful? http://julian.com/research/velocity/#dependencies

if not, let me know, and i'll dive deeper into your question

@Prinzhorn
Copy link

Thank you very much, it does indeed help and is working for now. But one issue remains: I don't want to mess with globals. Imagine using browserify to bundle an embed script (think disqus). It should not expose or require any globals at all to not mess with the host page. I'm now doing the following, since it looks like velocity only needs jQuery to be available globally once.

var oldjQuery = window.jQuery;
var old$ = window.$;

window.jQuery = window.$ = require('jquery');
require('velocity-animate');

window.jQuery = oldjQuery;
window.$ = old$;

Same problem with global variables when I use velocity without jQuery. Doing var Velocity = require('velocity-animate'); will at the same time also set window.Velocity.

I'm in fact not creating an embed script atm, that's just something to think about. Here's what I do for now which is OK for me.

var $ = require('jquery');
window.jQuery = window.$ = $;
require('velocity-animate');
delete window.jQuery;
delete window.$;

@julianshapiro
Copy link
Owner

got it. thanks for the heads up, Alexander. not sure which way to go at the moment..

Rycochet pushed a commit that referenced this issue Aug 3, 2020
Fixes CommonJS module loading. Closes #232. Closes #238.

Allow animating with the raw utility function (Velocity) instead of its
property (Velocity.animate), e.g. $.Velocity(element, propertiesMap,
options);

Copy over _cacheValues option for the UI pack. Closes #239.

Reverts Velocity.mock to original (asynchronous) behavior. Closes #237.

Fixes IE7 error throwing in UI pack. Closes #246.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants