JSM: Added dat.gui module version. #16754
Conversation
|
Do we want to have ES module copies of My first reaction would be that maybe |
I think it makes definitely sense for stuff like |
That's a good point. The current used version is It seems ES6 support was added with 0.7.0. The one I have added here is |
Ah, yeah that's a good distinction. I have no objection to converting inflate.js. Once we're generating UMD from ES modules, we should remove the old version of inflate.js.
I don't think we should be creating ES versions of external libraries that don't already have them (like Draco and Basis), so we can't go 100% on this... not having any more duplicates than necessary would be nice, too. |
All the libs, except Converting Perhaps we can reach out to the library owners and see what the status of ES6 support is? |
Um, this sounds like a time consuming task. I mean to wait until all relevant projects provide ES6 builds. Besides, they will also provide the latest build as a module and not the version used in In the meantime, I think it's more "target-aimed" to manually convert the modules similar to |
I'm fine with doing that, and it's generally fairly easy to do. We should add a comment to the start of each file that we edit this way though. |
This has considerably more side effects than just converting our source files to ES modules. Consider JSZip (74KB) or Ammo (1.6MB). I don't think either of these should be consumed by other modules, or not as module-relative dependencies anyway. MMDLoader depends on Ammo, but not all MMD files need it (as far as I know) and certainly not all users want that 1.6MB added to their build. And even for much smaller dependencies, it seems worth thinking twice before we locally edit and republish modules that are already available on NPM. We can certainly work through all of this, there are several options. But... this seems much lower priority to me. Could we leave unmodified global scripts for |
There are probably much smaller decompression utilities that we could replace this with, such as zip.js or files from zlib.js. Performance may be different though. Ammo.js I agree on, I don't think we should convert that to a module manually. Perhaps we can remove the dependency from
That's a fair point, it could cause complications for users or cause them to have some libs included twice by mistake if they are already using the NPM version.
That's fine by me, but that's probably going to be in just a few days at the rate @Mugen87 has been converting the files 😅 Aside from that, I already find it awkward in my workflow when I import say, the |
|
Sorry I haven't (and maybe won't because of lack of time) read though the discussion in this thread but one comment for MMD
Most of MMD model files include Physics settings and most of MMD users expect Physics works fine because Physics is one of the big features in MMD. |
Still, though... if any users are not relying on physics, surely they'd rather not have a 1.6MB physics engine bundled into the loader? Or even if they are, keep the option of lazy-loading the physics part? I don't think AmmoJS should be imported directly by the loader. Or to take another example, GLTFLoader depends on DRACOLoader, which depends on the 300KB Draco decoder module with JS and WASM variants. Those files should not be direct dependencies of GLTFLoader: (1) it's pointless to bundle both the JS and WASM decoders, (2) there's no bundler-agnostic way to load only the appropriate decoder, and (3) not all glTF models need Draco. In this case, providing the dependency is better:
I don't think it is that easy or simple, unfortunately. We haven't even gotten to the more important part of compiling ES modules to backward-compatible UMD files. I agree we need to solve the dependencies too (without relying on global names) but my hunch is that several existing modules would benefit from the |
True, but isn't figuring out how to handle the dependencies something we need to do before we proceed to that stage?
Yeah, I was thinking the same. That's possibly the least bad solution here, and if we do that then I see why you're saying we don't need to prioritize the libs yet. In the long term though, it's not great to have modules with dependencies that are not available as modules, so we should not ignore them completely. In the case of |
|
Just in case, to clarify. As I said I haven't read and won't jump in the discussion, and no opinion. But I just found the sentences about MMD which don't look right and I just wanted to correct. So please ignore if it isn't related to the center core of the discussion. And one more nit, the file which has dependency to Ammo is not MMDLoader, but MMDPhysics. Loading MMD doesn't require Ammo. Ammo is required if users want to do animation with Physics. (Please ignore too if it isn't important for the discussion). |
|
I vote to create a module version for mandatory dependencies (or dependencies which are necessary in most cases) like In context of optional dependencies I agree with @donmccurdy. Injecting them seems the better solution. Otherwise we end up with unnecessary overhead. This means we might want to introduce |
|
Let's shift further module related discussions to #16688. |
|
Thanks! |
And adjusted one of the examples so it uses the new code.