Examples: Fix and refactors using three's ESM addons

Desplandis commented 7 months ago

Description

While bumping our dependencies, I came across some examples using three's addons which were either not working or using a pre 0.148.0 version (before three's decision to remove the examples/js folder, i.e. their commonJS addons). This PR fixes both issues.

Motivation and Context

Fix of those issues involves:

using importmap scripts to import three and its addons
using module scripts to support ES6 syntax and imports (see Applying the module to your html))
as a bonus, using post-ES6 syntax (e.g. replace var by let/const)

I only done this on the three examples listed below but I think we should refactor other examples in the same way.

Impacted examples

Effects/Stereo effects (was not working)
View/Multiple 2.5D maps
Misc/Collada

Desplandis commented 7 months ago

@mgermerie I just saw another PR of yours (#2151) which attempts to fix this issue with the Collada example. This PR may close your PR.

Desplandis commented 7 months ago

@jailln

[...] we import threejs twice in these examples: once through itowns.js and once with import * as THREE from 'three';. It is not ideal but it is not that important either in the case of itowns examples and I think that users that will create applications where this would matter will import itowns as an npm module and therefore won't have this problem.

Yeah, I know but I didn't find a way to link itowns.THREE to module three with importmap. I don't think there exists an easy way to do such duck taping and I don't have time to look into the nooks and crannies of the esm module resolution... If anyone finds a way, I'll gladly update those examples.

Still, I think we sould still add a comment about this in the examples so users are aware of it.

Yeah, could do this.

Another solution [...] would be to create a static bundle without the peer-dependencies (in addition to the current bundle) and use it in these examples at least. Note that this solution has been discussed in #1802 some time ago and that I have a dirty implementation of this on the following branch).

Yeah, I considered such option too (thanks for the link to the discussion btw) but this implied to modify all our examples. Plus I don't know how a commonJS bundle (i.e. itowns) without its dependencies could reference esm modules mapped with importmap!

Desplandis commented 7 months ago

@jailln Since your review I added the following lines at the end of the examples:

            // Warning: the following code is not part of this example, those
            // variables are only exposed for internal functional test uses.
            window.view = view;

Since global variables in ES6 modules are limited to the scope of its module and we are using view on our functional tests (see test/functional_hooks.js), we must "export" them to the window scope. I added a notice for our users, WDYT?

If we were to convert all our examples, we should "export" all used variables/functions in the widow global scope.

jailln commented 7 months ago

Yeah, I considered such option too (thanks for the link to the discussion btw) but this implied to modify all our examples. Plus I don't know how a commonJS bundle (i.e. itowns) without its dependencies could reference esm modules mapped with importmap!

Yep you're right, it may not be straightforward. I agree with you it's not a priority and leaving it like this with a comment is fine by me.

Since global variables in ES6 modules are limited to the scope of its module and we are using view on our functional tests (see test/functional_hooks.js), we must "export" them to the window scope. I added a notice for our users, WDYT?

Fine by me too :+1:

iTowns / itowns