mmdparser: Convert to ES6 classes.

[CodyJasonBennett]

Related issue: #XXXX

Description

Converts the mmdparser lib to ES6 modules for tree-shaking. This was wrapped in an IIFE in #26912, but is otherwise not very complicated to refactor compared to other libs. There is a remaining lint warning for the use of throw 'str' instead of throw new Error( 'str' ) that I've left untouched.

[CodyJasonBennett]

Maybe the IIFE was a better idea.

[LeviPesin]

It would be so much easier if we could just import libs from their NPM packages (as done in three-stdlib), instead of needing to copy, maintain, format and fix them in the Three.js' repository... @mrdoob @Mugen87 Maybe we still could do that for libs used for addons?

[marcofugaro]

Yeah, not sure if modifing a built lib directly is the right approach, often it's copy-pasted from the source repo:
https://github.com/takahirox/mmd-parser/tree/master/build

It would be problematic on next updates. Maybe better to take the npm + unpkg.com route.

[CodyJasonBennett]

I've had to vendor some in three-stdlib as well for similar reasons but also packaging since some are misconfigured and don't resolve as either CJS or ESM when they should. three should mostly be unaffected from the latter issue. I can upstream these fixes and notify here when it's safe to consume.

[marcofugaro]

Also not sure how you're testing this, but MMDParser is tree-shakeable in my tests for esbuild, webpack and rollup.

Here is the test I did:

// node_modules/three/examples/jsm/loaders/MMDLoader.js

import { Loader } from 'three';
import { MMDParser } from '../libs/mmdparser.module.js';

class MMDLoader extends Loader {
	_getParser() {

		if ( this.parser === null ) {

			this.parser = new MMDParser.Parser();

		}

		return this.parser;

	}
}

class UsedClass {}

export { MMDLoader, UsedClass };

// src/index.js

import { UsedClass } from 'three/addons/loaders/MMDLoader.js';

console.log(UsedClass)

With all bundlers only the UsedClass is present in the final bundler.

[CodyJasonBennett]

I've been testing with npx agadoo@latest against addons from #26910, only trying individual tools to ensure correct build output.

Perhaps this isn't good enough. Prior, I setup https://github.com/CodyJasonBennett/treeshaking to test misc. behaviors.

[Mugen87]

I would like to continue to copy libs into the repository. Not all developers use npm to host the code. If we introduce real (non-dev) dependencies, we force all users towards npm.

Besides, certain libs perform too much logging or inconsistent error handling that contradicts three.js policies. In this case, it's more clean to modify the library to provide users a more consistent experience. Good examples for this are UTIF or UPNG which are embedded in loaders.

instead of needing to copy, maintain, format and fix them in the Three.js' repository

Looking back at 8 years of maintenance, upgrading third party libs was never an issue. The copy is fast, formatting happens automatically if wanted, and the number of third party libs is small. Besides, we never blindly upgrade dependencies and there is no pressure to upgrade to each patch or minor release.

[LeviPesin]

Good examples for this are UTIF or UPNG which are embedded in loaders.

We can continue packaging them, but why we couldn't use, for example, external fflate or mmd-parser?

[marcofugaro]

@CodyJasonBennett hmmm not sure about the inner workings of that tool, it is better to test with the bundler directly in my opinion.

I have setup a repo similar to yours if it's useful, with esbuild + rollup + webpack.
https://github.com/marcofugaro/treeshake-test

Make sure to import from the node_modules since bundlers treat app code differently from vendor code regarding tree-shaking. I often use yarn link.

[Mugen87]

@LeviPesin We can solve the tree-shaking issues without changing the policy how external libraries are managed.

[CodyJasonBennett]

I'll leave it to @takahirox as for whether this is preferable for code-style or other reasons, but for tree-shaking this PR proves unneeded and comically difficult to review. Thanks @marcofugaro for the assistance and direction.

[mrdoob]

@takahirox should we update the mmd examples to user the mmd-parser npm and unpkg?

[takahirox]

Regarding ES6, mmd-parser was written very long time ago. It's nice to rewrite in ES6. Probably doing it at the original repo https://github.com/takahirox/mmd-parser, and then copy or import is good. PR is very welcome.

Regarding npm or unpkg, I think it should be aligned with our external libs policy. If other libs are imported via npm/unpkg, mmd-parser should be, too. If they are not, mmd-parser should not be.