BabylonJS / Babylon.js

Babylon.js is a powerful, beautiful, simple, and open game and rendering engine packed into a friendly JavaScript framework.
http://www.babylonjs.com
Apache License 2.0
23.27k stars 3.44k forks source link

Latest npm package is not node friendly, not isomorphic, breaks Browserify/Webpack workflow. #539

Closed kevzettler closed 9 years ago

kevzettler commented 9 years ago

I upgraded my npm babylon dependency to the latest 2.0.2 and my project no longer works. I have been using the old 1.13.3 npm module with success and I can downgrade to that version and things still work.

I'm using webpack to require babylon in a browser environment. In the browser i'm now seeing this error Uncaught TypeError: Cannot read property 'prototype' of undefined

Which is coming from some minified babylon code that looks like:

        var __extends = this.__extends || function(t, e) {
            function i() {
                this.constructor = t
            }
            for (var r in e)
                e.hasOwnProperty(r) && (t[r] = e[r]);
            i.prototype = e.prototype, t.prototype = new i  ///******* THIS LINE IS THROWING ERROR****

Listed below is some output that shows problems when trying to require the babylonjs module directly form a node server side repl.

kevs-mbp:web kevzettler$ npm install babylonjs
kevs-mbp:web kevzettler$ npm ls babylonjs
rougeland-server@0.0.0 /Users/kevzettler/plebland/web
├── babylonjs@2.0.2

Ok we have 2.0.2 installed

^Ckevs-mbp:web kevzettler$ node
> var BABYLON = require('babylonjs');
ReferenceError: window is not defined
    at BABYLON (/dist/node_modules/babylonjs/babylon.js:3:24377)
    at Object.<anonymous> (/dist/node_modules/babylonjs/babylon.js:3:24459)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at repl:1:15
    at REPLServer.self.eval (repl.js:110:21)

Just requiring it initially throws an error because there is no window object. Its usually bad practice to publish code to npm that expects a window. Its okay though we can spoof it right?

> var window = {};
undefined
> var BABYLON = require('babylonjs');
ReferenceError: navigator is not defined
    at Function.r.GetPointerPrefix (/dist/node_modules/babylonjs/babylon.js:5:10268)
    at BABYLON (/dist/node_modules/babylonjs/babylon.js:10:27190)
    at Object.<anonymous> (/dist/node_modules/babylonjs/babylon.js:11:5056)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at repl:1:15

Same issue with the navigator object. This issues exists in the 1.13.3 I was already working around it. We can spoof it like window.

> var navigator = {};
undefined
> var BABYLON = require('babylonjs');
TypeError: Cannot read property 'prototype' of undefined
    at __extends (/dist/node_modules/babylonjs/babylon.js:20:25655)
    at /dist/node_modules/babylonjs/babylon.js:20:26207
    at BABYLON (/dist/node_modules/babylonjs/babylon.js:20:26224)
    at Object.<anonymous> (/dist/node_modules/babylonjs/babylon.js:20:26268)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)

Ok so now it breaks by just requiring the module. Is it possible spoofing window and navigator broke it? Hard to tell because the code is minified.

Firstly I think there should be better error capturing in this library. I've hit vague errors several times now that don't give any indication of whats breaking. Secondly I feel that the minified code should not be published to npm.

deltakosh commented 9 years ago

You also have the unminified version in the package: Babylon.max.js I'll fix this issue in npm asap

deltakosh commented 9 years ago

Can you do a new npm install to test the nez version?

kevzettler commented 9 years ago

It appears that the latest version 2.0.4 is not exporting any thing. require('babylonjs') returns an undefined object.

^Ckevs-mbp:web kevzettler$ npm ls babylonjs
rougeland-server@0.0.0 /Users/kevzettler/plebland/web
├── babylonjs@2.0.4

kevs-mbp:web kevzettler$ node
> var window = {}; var navigator = {};
undefined
> var BABYLON = require('babylonjs');
undefined
> BABYLON
{}
> BABYLON.Engine
undefined
deltakosh commented 9 years ago

I've just tested and the package is ok. If you do npm install babylonjs, you will get Babylon.js, Babylon.max.js inside node_modules folder

deltakosh commented 9 years ago

untitled

kevzettler commented 9 years ago

@deltakosh yes the package does include those files. I am seeing that installed to my node_modules directory as well. However that dosen't mean its a correct functioning npm module.

If you look at the 1.13.3 module you will see it is packaged with the following files

kevzettler@kevs-mbp /d/n/e/n/babylonjs> ls
README.md     babylon.js    hand-1.3.8.js package.json

looking at 1.13.3 babylon.js you can see the key line we are looking for. The last part of the file is: module.exports=BABYLON;

this is part of the CommonJS module spec. Which NPM uses. See: http://wiki.commonjs.org/wiki/Modules/1.1 This enables CommonJS module loaders to require the exposed, exported object.

deltakosh commented 9 years ago

Hum..a.for some reasons the module export was removed. I will add a module.exports= BABYLON to the final js file. Just need to tweak the gulp build for this:)

kevzettler commented 9 years ago

Sounds good.

I want to address your concern of:

"This is a client library why should we support node?"

First of all node is one of the most successful and widely used javascript platforms. Though node its self is a server environment, the CommonJS module specification is used for other client dependency management tools. So really this is asking for CommonJS module support.

Secondly. I am building out a multiplayer web based game. I would like to have reusable code between the client and server for things like: Collision detection, Vector math. Bounding Boxes, and Physics. Babylonjs already has most of that built in.

Are you suggesting that I should find other Libraries to handle those concerns specifically for the server? Am I incorrect in thinking Babylonjs handles those tasks? It sounds like you're asking me to duplicate the effort for the server.

deltakosh commented 9 years ago

No you're absolutely right. I wrote this and then thought about it. I should have done the opposite. This is why I removed my comment

RaananW commented 9 years ago

https://github.com/efacilitation/gulp-wrap-commonjs , this should be helpful. @deltakosh want me to look into implementing this extra task?

RaananW commented 9 years ago

And just a side comment - babylon has hand JS as dependency. should it have a package of its own? Or maybe Included in the package as well? (But then it won't be "requireable" as well...)

deltakosh commented 9 years ago

For hand.js, it is an optional dependency. So I prefer leaving it outside of the package. I would really appreciate if you could check this for me Raanan.

My goal was to create a footer.js in the gulp folder and concat it to unminified version. It has to contain the test for module existence and the export.

In the same time I was planning to remove all var __extends blocks added by TS and add only one by concatening a header.j's at the beginning

kevzettler commented 9 years ago

Hey guys I just want to give an example of a client distribution setup that I've seen in other modules. As part of your build you want to have something like a /dist directory that you can point bower and npm too.

/root
   package.json  #--> { main : dist/babylon.js}
   bower.json #--> {main : dist/babylon.min.js}
   README
   gulpfile.js #--> This builds to /dist
   /src
   /dist
     babylon.js
     babylon.min.js

You can check this out in https://github.com/peers/peerjs hopefully that helps

RaananW commented 9 years ago

just before I destroy something that works :-) : What that is actually missing is a module.exports=BABYLON at the end of the file. Of course making sure that modules is an object.

Regarding window/navigator - this is a frontend canvas based framework who requires information from those two. I actually had the same problem while using the framework in a worker. my solution was the same as yours. Maybe we could simply add a

window = window || {}
navigator = navigator || {}

At the start of the built file to solve those kind of problems.

@deltakosh I saw you used this gulp-clean typescript plugin, seems to work rather well. Is there something wrong with this plugin?

I will add the exports plugin soon, have to implement a quick gulp plugin myself as the others don't do 100% what we need. shouldn't take long.

deltakosh commented 9 years ago

yes something like:

if (typeof module === "object" && module.exports) {
 module.exports = BABYLON
}

Nothing wrong with gulp-clean :)

RaananW commented 9 years ago

so, finished writing everything and wanted to commit, but github doesn't really let me sync branches at the moment :-) PR Coming as soon as github decides I am worthy.

deltakosh commented 9 years ago

What error do you get?

RaananW commented 9 years ago

I couldn't login all night. They fixed it now. https://github.com/BabylonJS/Babylon.js/pull/553

kevzettler commented 9 years ago

@deltakosh @RaananW Has the latest version been published to npm? Just tried upgrading. Looks like i'm at 2.0.4 and has the same issues

kevs-mbp:client kevzettler$ npm ls babylonjs
client@1.0.0 /Users/kevzettler/plebland/client
└── babylonjs@2.0.4 

kevs-mbp:client kevzettler$ node
> var BABYLON = require('babylonjs')
ReferenceError: window is not defined
    at BABYLON (/Users/kevzettler/plebland/client/node_modules/babylonjs/babylon.js:3:24364)
    at Object.<anonymous> (/Users/kevzettler/plebland/client/node_modules/babylonjs/babylon.js:3:24446)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at repl:1:15
    at REPLServer.self.eval (repl.js:110:21)
> var window = {}; var navigator = {};
undefined
> var BABYLON = require('babylonjs')
undefined
deltakosh commented 9 years ago

Not yet. Still working on other issues

deltakosh commented 9 years ago

Should be goooooood now :)

kevzettler commented 9 years ago

@deltakosh @RaananW Looks like its still broken Heres 2.1.0

^Ckevs-mbp:client kevzettler$ npm ls babylonjs
client@1.0.0 /Users/kevzettler/plebland/client
└── babylonjs@2.1.0

kevs-mbp:client kevzettler$ node
> var BABYLON = require('babylonjs')
ReferenceError: window is not defined
    at /Users/kevzettler/plebland/client/node_modules/babylonjs/babylon.js:4:20149
    at BABYLON (/Users/kevzettler/plebland/client/node_modules/babylonjs/babylon.js:4:20288)
    at Object.<anonymous> (/Users/kevzettler/plebland/client/node_modules/babylonjs/babylon.js:4:21001)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at repl:1:15
> var window = {}; var navigator = {};
undefined
> var BABYLON = require('babylonjs')
undefined
> BABYLON
{}

Looks like there is actually an exports statement in the code now. Thats good. However it looks wrong.

kevs-mbp:client kevzettler$ cat node_modules/babylonjs/babylon.js | grep exports
window.module&&module.exports&&(module.exports=BABYLON);

What if there is no window.module ? it just fails at that clause and never sets the export. Which again there is no window in node.js and i'm not even sure what window.module is. I'm using webpack and thats not available.

If I remove window.module&& everything works as expected

RaananW commented 9 years ago

I don't remember this "window" being there... let me check :-)

RaananW commented 9 years ago

window reference was remvoed. @deltakosh needs to update npm with the new version and it should work.

BTW, the window and navigator references were not addressed, so you will still need to define both objects before using it.

deltakosh commented 9 years ago

Actually this IS required by IE I think..did you test on IE Raanan?

deltakosh commented 9 years ago

I fixed it..let me know if this is ok

RaananW commented 9 years ago

Nope, don't check on ie. Your fix seems the best way to support every browser.

RaananW commented 9 years ago

*Didn't. Silly keyboard...

deltakosh commented 9 years ago

:D

deltakosh commented 9 years ago

Fantastic

kevzettler commented 9 years ago

@deltakosh @RaananW Unfortunately I spoke to soon. I was looking at my modified version and had to reinstall/rebuild. This is still broken.

kev-win-pc:client kevzettler$ npm ls babylonjs
client@1.0.0 /Users/kevzettler/plebland/client
└── babylonjs@2.1.1

kev-win-pc:client kevzettler$ node
> var window = {}; var navigator = {};
undefined
> var B = require('babylonjs');
undefined
> B
{}

Heres your latest export statement. I don't even know whats going on here.

kev-win-pc:client kevzettler$ cat node_modules/babylonjs/babylon.js | grep exports
(window&&window.module||!window&&module)&&module.exports&&(module.exports=BABYLON);
> (window&&window.module||!window&&module)&&module.export
false
kevzettler commented 9 years ago

If I change the exports 'footer' to

(BABYLON||(BABYLON={}));';
(window&&window.module|| typeof window != 'undefined' && typeof module !=' undefined')&& typeof module.exports != "undefined"&&(module.exports=BABYLON);

It seems to work. some of the Truthyness checks around window and module are acting strange. I also removed the comma after (BABYLON||(BABYLON={})) i'll try and update the build but not sure where the commas coming from.

kevzettler commented 9 years ago

see https://github.com/BabylonJS/Babylon.js/pull/570

RaananW commented 9 years ago

What an adventure :-) The commas are coming from uglify2 and we have no control of them. If you tested that using the gulp-built babylon.js file it should work. I am currently away from my computer and can'T test it on all browsers. I am sure @deltakosh will be able to do that later.