-
Notifications
You must be signed in to change notification settings - Fork 174
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
Do not rely on THREE global #13
Comments
This already has a peer dependency on ThreeJS, it's just not explicit in the Explicitly requiring the Global scope isn't ideal but I'm not sure there is any solid solution yet. See more here: |
I ran across this issue as well, using webpack, and I had to dig around to find the webpack command to make it work. I'm posting it here in case other people had the same problem. In your webpack.config.js configuration object, add:
|
Running into this issue as well. We have a build that uses webpack and we're loading pre-bundled three from a lib folder because it's a modified three.js build. Including bmfont-text via npm will cause THREE to not be found, both using global or window object because it's all loaded at once in the bundle. Even using ProvidePlugin solution above does not work since THREE isn't installed via NPM. This was actually a huge headache for datguivr as well. We originally wanted to do tree shaking via rollup but this module couldn't find THREE either. Not exactly sure what the right solution for this lib is though :| It probably should include THREE ES6 module bits. The problem stems from immediately needing THREE to be in scope at
This happens as soon as an import statement from bundlers hit BMFontText. You literally can't declare window.THREE = THREE or global.THREE = THREE before this happens unless it takes place in a separate script. |
Indeed... seems like one solution would be to refactor this library to not alias THREE until it's first called. That wouldn't change the API for anyone else, but would allow for workarounds for scenarios like @mflux has. |
I made these quick changes to get this library working with ES6 / Browserify. It's not really PR ready as i had to pull in some other deps code directly, but just thought i'd reference it here.. The cleanest and safest way i've discovered so far for ES6 use for custom three modules is to reference three.js only as a proper peerDependency in the package.json. Similar to this ( https://github.com/jamieowen/three-material-modifier/blob/master/package.json ) But definitely don't reference three in dev or prod dependencies ( even for demo-making purposes ) as a version still gets put in the deps node_modules. Also you need to add NODE_PATH=./node_modules before your browserify/budo related commands otherwise node will complain about not finding three in your deps package.
I've had some weird shit happen when other dependencies include three.js, like declaring reusable Geometry in the top level scope of a js file causes some weird rendering problems that are basically impossible to debug as there are no errors, just nothing appears but only in certain circumstances.. confusing as hell! :) I haven't figured out why those problems happen exactly but this setup has really helped me avoid any issues. |
This does make it difficult to use with ES2015 modules though, as THREE needs to be set as a global, as a hack, somewhere before this module is imported, but due to the dependency graph, it's not that easy to do properly |
Has anyone gotten the library working inside Angular 5? Declaring THREE as a global variable (window.THREE = THREE or document.THREE = THREE) don't seem to do anything. I keep getting an error complaining THREE is not defined. I think a simple way to allow developers to make the library use whatever version they want of three.js is for the THREE object to be passed as an argument when calling the library. This approach would allow maintaining the flexibility of the library while also supporting modules on ES2015 and frameworks such as Angular and others. |
I went the slightly different and hackier route. I don't want to add ALL of 'three' to our bundle size (we've got some stripping going on), but we can forward the parts needed into the global three to allow the package to work without modifications.
|
I agree this is a pretty bad decision. There are hacks to go around it but that's all they are, hacks. I don't see what's the problem with importing THREE in the library's own file locally, since anyway that will default to whatever the version any given user has installed in |
this could be better. |
🤷♂️ ThreeJS has come a long way in terms of npm/modularity/build tools since this issue was opened (in 2016), so it makes sense to re-evaluate, and that discussion has already started in a PR here. But it's not really as simple as just accepting the PR as it stands now and making a breaking change, as it will likely break a lot of the software that is using this module in production. For example, AFrame is using this module, and (no surprise) they have forked ThreeJS and are using their own version in their library rather than relying on node_modules. So if I do include So yeah; the decision may appear poor now in hindsight but it's also more nuanced than it might appear, and times have change since 2016. 🍻 p.s. @wilson0x4d Commenting "this could be better" is a waste of my time as it goes to my inbox as an email. Maybe just fork this module and move on, or don't use my module at all if you don't agree with it. |
To me it sounds like it would be on whoever is curating that library to then take care that, if they fork THREE, they also fork modules like this one and make the necessary changes. It leaves me puzzled that this is the one case that controls the decision rather than the most straightforward one where I |
I am building my application using Webpack and started using ES6 modules to import three.js functions. Since this module relies on the THREE global, I ran into a problem that it is undefined inside this module.
It works for now by setting the global THREE manually. However, I think it would be nice to add three.js as a peer dependency on this module and import the required function from three.js inside, instead of relying on the global.
What do you think?
The text was updated successfully, but these errors were encountered: