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

npm #14

Open
cmloegcmluin opened this issue Feb 13, 2018 · 9 comments
Open

npm #14

cmloegcmluin opened this issue Feb 13, 2018 · 9 comments

Comments

@cmloegcmluin
Copy link

Any interest in making this awesome tool available as an npm package?

@stewdio
Copy link
Owner

stewdio commented Aug 3, 2018

Yes! Do you want to write the first draft of that? 😉

@cmloegcmluin
Copy link
Author

Already took care of it, actually! https://www.npmjs.com/package/three-vrcontroller-module

@Kif11
Copy link

Kif11 commented Mar 28, 2019

@cmloegcmluin Thanks a lot! How did you do it? Is it possible to automate?

@cmloegcmluin
Copy link
Author

This is the commit that did it: cmloegcmluin@ec8c720#diff-8d6f5c826e622ae81907df0290f3fa04

Automation should be possible. The key changes are:

  1. at the top, importing from THREE rather than expecting THREE to already exist in the global scope
  2. at the bottom, exporting the VRController rather than patching the global THREE.

The rest of the changes are not important. In fact, if you were automating this, you'd probably want to simply import * as THREE. That would have a couple benefits.

  1. The rest of the file could stay the same.
  2. You wouldn't have to do the step of finding the list of only the exports of THREE that you need for that module, as I have done for VRController.

Good luck and let me know if you have any other questions.

(By the way, a bunch of my VR Three.js stuff stopped working in Chrome this week due to a change in the WebXR API: navigator.xr.requestDevice is not a function. In fact, all of the WebVR examples at good old https://threejs.org/examples/ are down for this same reason (not surprised since I modeled my code after one of them)).

@cmloegcmluin
Copy link
Author

Also, I noticed that the GitHub URL for my npm package was out of date, and have corrected it. I apologize if you had already gone looking for this information and I didn't make it easy on you!

@Kif11
Copy link

Kif11 commented Mar 29, 2019

Cool! Thanks for detailed explanation. I will probably use your fork instead because I really don't get an idea of assuming THREE in global scope. I might open an PR to you repo since I found some functionality missing from the library. #25

@cmloegcmluin
Copy link
Author

Sure thing. Happy to help.

I'll try to explain the whole global vs. module issue. THREE relies on the order you import files, such as with <script/> tags in your .html. You need to make sure you import the core THREE library first because that's the JavaScript file that will declare the variable THREE, and declare it scoped not to the file with a var, let, or const, but globally - essentially declaring it to be a property of window or global, as in window.THREE. The other THREE files, like this VRController one, assume you have already done this, and will not work correctly if you haven't.

It's more popular in the community nowadays to use "dependency injection" style, where each JavaScript file states explicitly what it needs to work correctly in the form of require or import statements. It is considered a cleaner and more scalable way to code. It's how package libraries like npm expect you to serve your code. I won't delve into the rationales here, but I hope this helps clear up my original intention and my process.

I'm not surprised if functionality is missing - the way I did this was super-dumb - the code is just a snapshot of how it was the last time I was here. There may be some tricky way to build something that perpetually serves the latest version of THREE as modules without maintenance, but until then, PR's are no problem.

@Kif11
Copy link

Kif11 commented Mar 30, 2019

Yes I understand the <script> tag and the whole issue with implicit dependencies on the web. Mb at the time it was design it was right decision to make but right now it just seams wrong to have implicit dependencies. I like Webpack and NPM ecosystem even though it has it's own challenges. Thanks again.

@cmloegcmluin
Copy link
Author

Word. For example I recently had to bin hours of work when I couldn't resolve a circular dependency - a problem I could've pretty easily solve in the old world.

PR whenever you need!

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

3 participants