[WIP] Convert to ESM and ES classes #438

trusktr · 2021-08-31T05:55:48Z

Do the similar for roslib. Converted to ESM in the roslib PR.
Make GitHub releases contain build artifacts (or open a new issue for it and consider the task here done).

fixes #314

…o ES2015 classes, and replace `src/` with them.

…rt class field syntax so it won't fail on it

…omatically use

Gruntfile.js

* develop: Update Build Export OcTree (RobotWebTools#417)

…ported in every modern browser

trusktr · 2021-09-11T05:03:39Z

@MatthijsBurgh Merged develop, updated eslint (no errors on class fields or for-of), tests passing, build updated.

trusktr · 2021-09-11T05:05:51Z

My longer-term plan is

convert to TypeScript (which is compilable to WebAssembly with AssemblyScript)
once I finish GLAS (a port of Three.js to AssemblyScript) and asdom (DOM bindings for AssemblyScript) then I can compile ros3d to Wasm.

trusktr · 2021-09-11T05:09:17Z

rollup.config.js

@@ -3,16 +3,15 @@ const rollup = require('rollup');
 // plugin that transpiles output into commonjs format
 const commonjs = require('@rollup/plugin-commonjs');
 // plugin that transpiles es6 to es5 for legacy platforms
-const buble = require('@rollup/plugin-buble');


The code output is cleaner without Buble, and all browser nowadays support all the syntax being used (I've been using these ES module output at NASA with no issues). This eliminated errors Buble was having on newer syntax that I didn't want to spend time configuring (by wasting time adding plugins to acorn) when all browsers already support the syntax out of the box.

…tEmitter2 into the repo shims/ folder and convert it to an ES Module). It would be better to use something else other than EventEmitter2, which is 1600 lines of needless code just for an event pattern. @lume/eventful is only 100 lines of documented code.

trusktr · 2021-09-11T21:37:19Z

I updated the code to be compatible with native ES Modules (and it now requires a very minimal import-map.json at runtime). To do this, I had to move the needlessly-large EventEmitter2 class into the shims/ folder (I suggesting replacing those 1600 lines of code with the 100 lines of code (half of which is documentation) of the Eventful class from @lume/eventful, but that would be for a breaking release).

The .js file extensions in the import statements are needed for two reasons:

native ES Module systems (f.e. web browsers) do not automatically append .js extensions to import specifiers. Import specifiers in browsers are simply URLs relative to the current file. Importing path/to/foo will not automatically make a network request for current-file-folder/path/to/foo.js which means trying to download current-file-folder/path/to/foo will fail if that file doesn't exist on the server
static file servers could be modified to automatically look for a file after adding the .js extension in their implementation, but in practice the average static file server does not do this (they are not js-specific static file servers).

So, by placing .js file extensions in the import specifiers, this makes it easiest to get things working in a native ES Module system without the following workarounds:

add a build step that appends .js to import specifiers, or
use a special server that appends .js extensions during it's file lookup.

trusktr · 2021-09-11T22:18:38Z

Still need to run examples to make sure I didn't break something.

trusktr · 2021-09-11T23:00:40Z

Added a TODO for roslib. It is not yet classes, and not yet ES Module friendly. I'll do the same thing there.

trusktr · 2021-09-11T23:17:29Z

roslib got in the way of making this native ES Module friendly. I am going to update roslib to ES Modules first. The good thing is ES Modules are here to stay, officially.

trusktr · 2021-09-12T03:13:21Z

WIP PR in roslib: RobotWebTools/roslibjs#475

It is possible we can complete this PR without the roslib PR (it just won't have browser-ESM compatible source files, but the built .esm.js bundle will work).

…that global addons will be able to modify ROSLIB

… which caught a few errors

…s/roslibjs#475

…llation from git (see RobotWebTools/roslibjs@ee2d985 for more info)

…pare script (see previous commit) will ensure that the library is built during install, or users can use an unpkg.com URL like https://unpkg.com/browse/roslib@1.1.0/build/roslib.js for the global var script approach

trusktr · 2021-10-01T03:04:20Z

What are your thoughts on that last commit (ignore build output)?

f56c58a

Please let me know if that's ok or if I should keep build output (the reasoning is in the commit message).

…asses * upstream/fix_440: Fix RobotWebTools#440, correctly delete old grid Add missing semi-colon Update Build Bump rollup from 2.56.3 to 2.57.0 (RobotWebTools#443) Bump mocha from 9.1.1 to 9.1.2 (RobotWebTools#442) Bump @rollup/plugin-node-resolve from 13.0.4 to 13.0.5 (RobotWebTools#441)

MatthijsBurgh · 2021-10-04T08:27:04Z

What are your thoughts on that last commit (ignore build output)?

f56c58a

Please let me know if that's ok or if I should keep build output (the reasoning is in the commit message).

I think that is OK. I think a repo should never contain compiled files. But these were already there, when I joined. So I kept them.

Might be good that the build files are added as artifacts to releases.

trusktr · 2021-10-05T00:45:43Z

Might be good that the build files are added as artifacts to releases.

That's true. For that, I think we can automate it.

Make GitHub releases contain build artifacts (or open a new issue for it and consider the task here done).

trusktr · 2021-10-05T00:48:24Z

src/models/MeshResource.js

+    options = options || {};
+    var path = options.path || '/';
+    var resource = options.resource;
+    var material = options.material || null;


This variable is not being used anywhere. I noticed as I was looking at build output that Rollup strips the var material so it becomes a floating expression (not used for anything, but the expression remains because there could be an unknown side effect from reading the property).

src/models/MeshResource.js

…, or they end up installing a second version of Three.js which can lead to issues where singletons are expected, or they'll copy and fork a local ros3d lib, etc.

…not work inside of them

trusktr · 2021-10-05T21:28:22Z

src/visualization/interaction/OrbitControls.js


-    function onTouchEnd(event3D) {
+    const onTouchEnd = (event3D) => {


This fixes a bug: the functions were passed unbound as event handlers further below, so this would not work inside of them.

We need unit tests! ros3d is hardly covered.

trusktr · 2021-10-06T01:20:55Z

src/shims/three/core.js

+// (I've seen this problem happen in real-world ROS3D projects, especially with
+// roboticists or data scientists that are inexperienced with web
+// development).
+globalThis.THREE = THREE;


While we're at it, this solves issues like #314 by allowing people to use ROS3D's THREE, rather than them trying to import an external THREE instance and causing issues. In a following update, this should be improved to allow more use cases such as supplying THREE from outside or as a peer dependency via npm.

trusktr · 2021-10-06T03:12:50Z

Tests are passing in Node 16. There appears to be some difference between ESM in NOde 14 and 16 that makes tests fail in Node 14.

We can try to fix it, or we can also move forward with Node 16 (this whole change will be a major version bump anyway).

https://github.com/trusktr/ros3djs/runs/3809448165?check_suite_focus=true

* Widen package requirements As suggested in RobotWebTools/roslibjs#437 (comment) * Fix wrong socket.io version

haoming29 · 2022-11-11T20:14:00Z

Any updates regarding this PR? Thanks

MatthijsBurgh · 2023-02-07T14:31:20Z

Stale

trusktr added 5 commits August 30, 2021 21:33

Use the transpileToEs6 function to compile classes from ES5 classes t…

10b2846

…o ES2015 classes, and replace `src/` with them.

fix broken output from es6-transpiler

717447c

fix most lint errors from es6 conversion. TODO update eslint to suppo…

bdbe6a7

…rt class field syntax so it won't fail on it

remove es6-transpiler

d4740d3

add .editorconfig to tell people's editors what format options to aut…

b15fac7

…omatically use

trusktr changed the title ~~Convert to es classes~~ Convert to ES classes Aug 31, 2021

trusktr commented Aug 31, 2021

View reviewed changes

Gruntfile.js Outdated Show resolved Hide resolved

This was referenced Aug 31, 2021

Bump three from 0.89.0 to 0.128.0 #381

Closed

Export OcTree #417

Merged

trusktr added 2 commits September 10, 2021 14:24

Merge branch 'develop' into convert-to-es-classes

722af7c

* develop: Update Build Export OcTree (RobotWebTools#417)

delete es6-support, it is no longer needed

cd8afba

trusktr force-pushed the convert-to-es-classes branch from 89f0bb6 to cd8afba Compare September 10, 2021 21:25

trusktr added 3 commits September 10, 2021 17:17

remove buble because it is choking (by default) on syntax that is sup…

9984ec1

…ported in every modern browser

commit build output (it is cleaner without Buble, nice)

97ec1fb

update eslint, get tests passing, update build

6a3c279

trusktr commented Sep 11, 2021

View reviewed changes

trusktr added 3 commits September 11, 2021 14:29

update build

67eaa6c

ignore the .vscode folder (for people who use the VS Code editor)

2ae35d9

add roslib to shims/ with a TODO to make it ES Module friendly

1ddbccc

trusktr mentioned this pull request Sep 12, 2021

[WIP] convert to ES Module format RobotWebTools/roslibjs#475

Closed

5 tasks

trusktr marked this pull request as draft September 12, 2021 03:17

trusktr changed the title ~~Convert to ES classes~~ [WIP] Convert to ES classes Sep 12, 2021

trusktr added 6 commits September 30, 2021 19:45

make the ROSLIB import a modifiable object like we do with THREE, so …

9205588

…that global addons will be able to modify ROSLIB

ignore lock files, libs don't need them

a47c609

convert to type:module in package.json, add an esm test using Webpack…

f1eff92

… which caught a few errors

use the ESM version of roslib from the WIP pull request, RobotWebTool…

a0fd39b

…s/roslibjs#475

ensure tests pass during publish, and make the repo friendly to insta…

7130835

…llation from git (see RobotWebTools/roslibjs@ee2d985 for more info)

trusktr changed the title ~~[WIP] Convert to ES classes~~ [WIP] Convert to ESM and ES classes Oct 1, 2021

trusktr added 4 commits September 30, 2021 20:35

update temporary roslib commit hash

1efd5ad

add .npmignore

b9af9f5

tidy up, move shims/ folder into src/

cf4a20a

trusktr commented Oct 5, 2021

View reviewed changes

src/models/MeshResource.js Show resolved Hide resolved

MatthijsBurgh mentioned this pull request Oct 5, 2021

Add build files to releases #451

Open

trusktr added 2 commits October 5, 2021 14:07

Expose THREE as a global, otherwise people can't access Three.js APIs…

5ba4686

…, or they end up installing a second version of Three.js which can lead to issues where singletons are expected, or they'll copy and fork a local ros3d lib, etc.

fix: unbound functions were passed as event handlers thus this did …

1abbdc8

…not work inside of them

trusktr commented Oct 5, 2021

View reviewed changes

trusktr mentioned this pull request Oct 6, 2021

Issue using GLTFLoader and multiple THREE instances #314

Open

trusktr commented Oct 6, 2021

View reviewed changes

This was referenced Nov 6, 2021

Upgrade three js #460

Closed

generic urdf model loading datapartyjs/teleop-xyz#7

Merged

[wip] Upgrade to threejs 134 - after ES classes #464

Closed

k-aguete pushed a commit to k-aguete/ros3djs that referenced this pull request Oct 21, 2022

Widen package requirements (RobotWebTools#438)

5f20a0d

* Widen package requirements As suggested in RobotWebTools/roslibjs#437 (comment) * Fix wrong socket.io version

MatthijsBurgh closed this Feb 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Convert to ESM and ES classes #438

[WIP] Convert to ESM and ES classes #438

trusktr commented Aug 31, 2021 •

edited

Loading

trusktr commented Sep 11, 2021

trusktr commented Sep 11, 2021 •

edited

Loading

trusktr Sep 11, 2021

trusktr commented Sep 11, 2021 •

edited

Loading

trusktr commented Sep 11, 2021

trusktr commented Sep 11, 2021

trusktr commented Sep 11, 2021 •

edited

Loading

trusktr commented Sep 12, 2021 •

edited

Loading

trusktr commented Oct 1, 2021 •

edited

Loading

MatthijsBurgh commented Oct 4, 2021

trusktr commented Oct 5, 2021

trusktr Oct 5, 2021 •

edited

Loading

trusktr Oct 5, 2021

trusktr Oct 6, 2021 •

edited

Loading

trusktr commented Oct 6, 2021

haoming29 commented Nov 11, 2022

MatthijsBurgh commented Feb 7, 2023


		function onTouchEnd(event3D) {
		const onTouchEnd = (event3D) => {

[WIP] Convert to ESM and ES classes #438

[WIP] Convert to ESM and ES classes #438

Conversation

trusktr commented Aug 31, 2021 • edited Loading

trusktr commented Sep 11, 2021

trusktr commented Sep 11, 2021 • edited Loading

trusktr Sep 11, 2021

Choose a reason for hiding this comment

trusktr commented Sep 11, 2021 • edited Loading

trusktr commented Sep 11, 2021

trusktr commented Sep 11, 2021

trusktr commented Sep 11, 2021 • edited Loading

trusktr commented Sep 12, 2021 • edited Loading

trusktr commented Oct 1, 2021 • edited Loading

MatthijsBurgh commented Oct 4, 2021

trusktr commented Oct 5, 2021

trusktr Oct 5, 2021 • edited Loading

Choose a reason for hiding this comment

trusktr Oct 5, 2021

Choose a reason for hiding this comment

trusktr Oct 6, 2021 • edited Loading

Choose a reason for hiding this comment

trusktr commented Oct 6, 2021

haoming29 commented Nov 11, 2022

MatthijsBurgh commented Feb 7, 2023

trusktr commented Aug 31, 2021 •

edited

Loading

trusktr commented Sep 11, 2021 •

edited

Loading

trusktr commented Sep 11, 2021 •

edited

Loading

trusktr commented Sep 11, 2021 •

edited

Loading

trusktr commented Sep 12, 2021 •

edited

Loading

trusktr commented Oct 1, 2021 •

edited

Loading

trusktr Oct 5, 2021 •

edited

Loading

trusktr Oct 6, 2021 •

edited

Loading