julianshapiro / velocity

Accelerated JavaScript animation.
VelocityJS.org
MIT License
17.29k stars 1.56k forks source link

Can't require velocity within node.js due to document.createElement calls #474

Open KyleAMathews opened 9 years ago

KyleAMathews commented 9 years ago

I'm building an isomorphic site using React.js and Velocity for some animations. I'm running into a problem however when rendering pages on the server, pages that require Velocity throw errors on the server due to document not being defined.

ydaniv commented 9 years ago

why should you require Velocity on the server?

KyleAMathews commented 9 years ago

In order to render my React pages on the server.

ydaniv commented 9 years ago

Velocity is used for animation, so I'm confused here. How do you use it for rendering?

KyleAMathews commented 9 years ago

It isn't used for rendering on the server but as I'm using commonjs modules, it still has to be required server-side so that Webpack can bundle it together for sending to the client.

ydaniv commented 9 years ago

@KyleAMathews have you tried simply patching this module and replace all requires with a mock?

Polyrhythm commented 9 years ago

The solution I've had to use repeatedly in other open-source projects used isomorphically is adding an early return in a conditional that checks for window or document or some such. Honestly, this is going to get worse and worse as more people attempt isomorphic apps, so it's probably a good thing to keep in mind.

It would be great if I didn't have to make my own fork of this that is node-compatible. I can't conditionally import modules in my codebase so everything the client uses, the server must also at least be able to require.

dantman commented 9 years ago

@KyleAMathews

It isn't used for rendering on the server but as I'm using commonjs modules, it still has to be required server-side so that Webpack can bundle it together for sending to the client.

I'm using React in a similar way (but with browserify). But I just move call require calls for client-only stuff into the actual client-only methods that use them.

Alternatively require('is-browser') would work.

There's a plethora of client-only js libraries that probably will never fix themselves to mock load server-side. And the idea of loading a client-only module server-side that you don't actually use sounds terrible from what I think should be a best practice. So honestly I think the solution is going to have to be something generic, probably related to the loaders. Instead of going around telling libraries they should make their library a no-op when window/document aren't available.

KyleAMathews commented 9 years ago

Webpack doesn't support conditional requires like browserify does afaik.

Agree that a general workaround would be nice.

ydaniv commented 9 years ago

@KyleAMathews (asking again) have you tried patching the require() of Velocity, e.g. by using proxyquire?

andrewburgess commented 9 years ago

Just to toss in an additional usage case and show how it might be integrated in a React application: https://gist.github.com/tkafka/0d94c6ec94297bb67091

Something like that should be able to be rendered on the server (the functions componentWillEnter and componentWillLeave that trigger the animation are only called from the client side).

sogko commented 9 years ago

@KyleAMathews

I have a similar use-case (using velocity-animate in an isomorphic app). You can use conditional require in webpack, but you have to create two separate webpack bundles:, you just need to mock the library

Here's what I've done:

The javascript

var Velocity;
if (typeof window !== 'undefined') {
  Velocity = require('velocity-animate');
} else {
  // mocked velocity library
  Velocity = function() {
    return Promise().resolve(true);
  };
}

// code that uses the library below
...
psoares commented 9 years ago

578 I created seems to be related to this issue too. Can't do dojo builds without code modifications, because dojo builds are created on the server through nodejs or rhino.

Rycochet commented 8 years ago

From a quick look:

Most of the createElement calls are relating to IE8 support. There is one used to help determine browser prefixes. If the prefix detection gets pulled into a separate module and does feature detection then that should clean up things nicely.

alampros commented 7 years ago

For those interested, I was able to get this working by using the (modified) shim (similar to the above solution) from velocity-react and using patched versions of velocity.js and velocity-ui.js.

pizzarob commented 7 years ago

Something I did that worked well was use webpack's code splitting inside of React's componentDidMount lifecycle method. componentDidMount does not execute server side.

  componentDidMount() {
    import('velocity-animate').then(Velocity => {
      this.velocityScroll = () => {
        Velocity(this.selection, 'scroll', {
          duration: 1000,
          easing: 'easeOutExpo',
        });
      };
    });
  }

https://webpack.js.org/guides/code-splitting-async/