search

xeokit / xeokit-sdk

Open source JavaScript SDK for viewing high-detail, full-precision 3D BIM and AEC models in the Web browser.
https://xeokit.io
Other
715 stars 286 forks source link

Import bug in the XKTLoaderPlugin #118

Closed barnabasmolnar closed 5 years ago

barnabasmolnar commented 5 years ago

Hello there,

We've recently tried to use the XKTLoaderPlugin in our React application and came across a bug.

The way the pako package is imported in the XKTLoaderPlugin.js file won't really work with bundlers, I believe. At the very least, it doesn't work in our environment bootstrapped with create react app (which uses webpack under the hood).

Here is the issue insofar as I understand it:

The current import looks like this:

import './lib/pako.js'

What this actually does or does not do in this case is that it does not actually import anything. Please refer to the MDN docs here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#Import_a_module_for_its_side_effects_only

So when later in the plugin we have references to pako, such as: pako.inflate, etc, it will throw the following error:

Uncaught ReferenceError: pako is not defined

One solution would be to simply use a named import:

import pako from '.lib/pako.js'

However that is not going to work in a browser environment as the pako package itself is not an ES6 module and does not have a default export.

And since all your examples are in a browser environment, I'm guessing you'd rather not rewrite them all if you can help it.

I propose the following:

import * as p from "./lib/pako.js"
const pako = window.pako || p

What this does is it inserts pako into the current scope as p, containing all the exports from the module.

Now, depending on which environment we are in, we assign pako either to the pako object on the window object or to this exported p object. The former works because the pako lib puts pako on the global scope (which is the window in browsers), and the latter also works because we now have a reference to the pako object in the current scope.

I'll shortly prepare a PR for this, I'll let you know as soon as I have it ready.

In the meantime, I just wanted to explain the issue as best as I could.

barnabasmolnar commented 5 years ago

Please see the PR here: https://github.com/xeokit/xeokit-sdk/pull/119

xeolabs commented 5 years ago

PR merged