Three.js: Modules (or circular dependencies) broken in 85?

[drcmda]

Description of the problem

Using anything in THREE, even if it's just a Vector

import { Vector3 } from 'three/src/Three';
console.log(Vector3)

causes:

vendor.js:53016 Uncaught TypeError:
    __WEBPACK_IMPORTED_MODULE_0__Vector3__.a is not a constructor

for instance somewhere in Quaternion, which relies on Vector3.

    setFromUnitVectors: function () {

        // http://lolengine.net/blog/2014/02/24/quaternion-from-two-vectors-final

        // assumes direction vectors vFrom and vTo are normalized
>>>>>>> var v1 = new __WEBPACK_IMPORTED_MODULE_0__Vector3__["a" /* Vector3 */]();

It looks like for some reason it can't resolve circular dependencies any longer, the internal require returns an empty object. It breaks r85 for module users. The only change i made was updating from 84 to 85.

Three.js version

• [x] r85

Browser

• [x] Chrome

OS

• [x] macOS

Build tool

• [x] Webpack 2.4.1

[Mugen87]

What happens if you change this line https://github.com/mrdoob/three.js/blob/dev/src/math/Quaternion.js#L354 to just: var v1;? There was actually a change in r85 so some helper objects are created in the forefront. See https://github.com/mrdoob/three.js/commit/563060ae23cc8602b84588e89a49a5c17428fc6c

[drcmda]

I think there should be nothing wrong it, it looks correct, it's just that nothing gets resolved for some strange reason. If i revert the patch you linked it just complains somewhere else.

[Mugen87]

The mentioned commit is part of a larger PR, see #10627

That's currently to only source of your error i can image. Let's wait what others say.

[zorro-fr24]

I had encountered this issue a couple of day ago too, which forced me to roll-back to r84

But I did debug it enough to determine that as @Mugen87 also suggests rolling back the Quaternion v1 Vector3 instantiation does solve the problem ( you also need to do the same for all similar declarations in the Vector3 class, like in the project() and unproject() functions ). Depending on transpiling options, both babel and webpack loaders are returning empty objects from __WEBPACK_IMPORTED_MODULE_0__Vector3__["a" /* Vector3 */](); at the point setFromUnitVectors is first called.

This doesn't really make sense.

I'm guessing we should really raise this in the webpack issue tracker
although perhaps it's related to: babel issue 5654 and webpack issue 4753

[drcmda]

@mrdoob @TristanVALCKE

@joe-fr24 is it really just the quaternion? It looks like this was part of a bigger patch that goes through large parts of the codebase, like it would just complain about something else if Vector3 is reverted. And can we be really sure this is webpacks/babels fault since it worked in 84?

[zorro-fr24]

@drcmda nope it's not just quaternion:

revert closure optimisations pr

This works for me.

But rather than revert those otherwise sensible optimisations though, perhaps there's a way we can "have our cake and eat it" - perhaps @TristanVALCKE has a suggestion since I've not run the benchmarks

[mrdoob]

@zorro-fr24 would you like to do a PR here in the meantime?

[zorro-fr24]

@mrdoob no problem: #11246

[pailhead]

I found a circular dependency that broke my jest tests but it seems to be working otherwise. I'm still new to this type of testing/build systems so i'm not sure what the problem is. My case failed with LineSegments

[Mugen87]

Closing. The original problem was solved with #11246. Other problems in context of circular dependency should be reported to new issues.

[zorro-fr24]

FWIW webpack has released version 2.5.1 which resolves the original issue by correctly hoisting exports above imports ( https://github.com/webpack/webpack/issues/4753 ).

This works with r85.2 in my tests

Hopefully this means we won't run into future recurrences.

[pailhead]

I think my webpack setup could handle this stuff with no problems, but node couldn't. I'm not super familiar with this stuff, but i've been told that node is harmony compliant, and webpack does more on top of that. Hence webpack could handle the Line import, but jest couldn't.

[mrdoob]

FWIW webpack has released version 2.5.1 which resolves the original issue by correctly hoisting exports above imports ( webpack/webpack#4753 ).

Does this mean that we can now revert #11246?

[zorro-fr24]

Does this mean that we can now revert #11246?

@mrdoob Well I would suggest not - although the very latest revision of webpack 2.x will work now I think, those who still use the 1.x line will probably have issues.

Also consider that Babel has an open issue that looks suspiciously similar. Maybe we can keep an eye on it for r87 ;-)