DRAFT: Testing for Manifold 3D without top level async calls.

[tonious]

This is just a test against Manifold PR #1375.

• Removed all of manifold but the worker.ts, worker-wrapper.ts and editor.d.ts.
• Added a local pack of Manifold from the above PR, and updated imports.
• Removed async initialization code -- above PR does it via lazy loading.
• worker-wrapper.ts still calls setWasmUrl().

I've tried this locally and it seems to work, but I've been known to miss the perfectly obvious. Please let me know what you think!

[Loosetooth]

Nice, thanks!

I checked the changes, looks good to me.
Also tried the extension in development mode, seems to work just fine.

Once manifold-3d is released with the required /lib files, we can adapt this PR and merge it.

One more question though:
right now we still have the worker.ts and worker-wrapper.ts files in the extension.

Are these identical to the ones in the manifold-3d package?
If so, could we just import these worker files from the manifold-3d package as well?

[tonious]

Thanks for checking this. And writing this extension! I'm already using it myself 😁

Even though it's explicitly among @elalish's use cases, I didn't really anticipate other projects using the web worker as is. That's why I'm thrilled to see this project -- it's using manifold differently than I do, and that forces me to check my assumptions.

So, worker.ts is identical. worker-wrapper.ts has some initialization code (wrt: setWasmUrl) that doesn't exist upstream. Both files are present in the Manifold repo, but not the NPM package yet. I want to play with those a little more before trying to package a 'canonical' version.

My feeling is that worker-wrapper.ts will almost always require customization, which implies it shouldn't be packaged. However, that's an opinion unencumbered of any thought process.

What do you think? You're the first developer to consume this refactored API. Where do you feel the boundary should be?

[Loosetooth]

OK, I just went through the existing wasm bindings in the manifold repo.

Here's my feedback,

1. I believe worker.ts should be renamed to worker-utils.ts or something similar.
   It only contains helper functions, and this file is not actually used to create a worker.

2. The worker-wrapper.ts file should be renamed to worker.ts, as this is the file that is actually used to build the Worker.

3. If we decide to publish the old worker-wrapper.ts file (which I propose to be worker.ts), it should be adapted to handle an 'initialize' message like I do here in the VS Code extension. This makes it clear that if a developer wants to use the "default ManifoldCAD worker", that they can use that message to point the worker to the correct wasm URL.
   Actually, I do not understand how the current worker functions without the URL being set.
   If the developer wants to do some custom stuff in their worker, they can always just use the exports from worker-utils.ts and write their own.

The other option is of course to just not publish that file, in that case anyone trying to do the same as manifoldCAD will just have to take a look at the source code, which I guess is fine too.

[elalish]

I don't have any strong opinions here, but I'm all for publishing as much useful code as we can in our npm package. @Loosetooth, your suggestions sound reasonable to me, so @tonious if you agree, that's all good. However, I would say this is worth deciding on and implementing before we push our next npm release.

[tonious]

Almost ready to get rid of src/webview/src/wasm entirely. Just have to do something about editor.d.ts.

[elalish]

@Loosetooth did you see @tonious ' latest PR into manifold implementing your recommendations? Do you think we're at a good place for an npm release now? It's a significant expansion on what we've been publishing, so I'd like to get some eyes on it and make sure it's in good shape.

[tonious]

And now src/webview/src/wasm is completely gone. Type definitions are (hopefully) being loaded from the local npm version of Manifold.

As always, this is your codebase, and I don't know if I did that right :)

[Loosetooth]

And now src/webview/src/wasm is completely gone. Type definitions are (hopefully) being loaded from the local npm version of Manifold.

Nice!

As always, this is your codebase, and I don't know if I did that right :)

It's not correct yet.
In registerManifoldTypes.ts

the way it is implemented now, it directly reads some files from the node_modules folder, which is not available at runtime (inside a packaged vscode extension).

We should somehow get the strings from the type files at build time, and bundle them. (This would mean that the outer vscode-extension package should have manifold-3d as a dependency, and somehow imports and uses the raw type file strings.)
Perhaps it is not the best idea to have manifold-3d as a dependency of the vscode-extension package AND of the "inside" webview package at the same time. But it should work.

Another option would be to have the src/webview package create the types file on build, and have it ready in its built output folder. The vscode-extension can then read it from the /media folder just like it happens in the current 'production' branch.
In order to create the actual type file, the manifold-3d/lib/types-example.d.ts files need to be read as string, and somehow combined. Not sure yet how this can be done.

Let me know if anything is unclear or you need further help with this.
Happy to look into it.

[Loosetooth]

@Loosetooth did you see @tonious ' latest PR into manifold implementing your recommendations? Do you think we're at a good place for an npm release now? It's a significant expansion on what we've been publishing, so I'd like to get some eyes on it and make sure it's in good shape.

Perhaps it's good to wait with a review (and release) until we have this PR working with all the necessary files imported from manifold-3d.
If we know that it works here, that will already be a good sign.

[tonious]

I kind of split the difference. The webview build copies the relevant type files into media/types. Those are included in the extension, and registerManifoldTypes.ts builds the final type file on demand.

[Loosetooth]

Yes, perfect.

I just tried this out, works fine.

So with the exception of gl-matrix.d.ts, all of the type files that were copied from manifold have now been removed.
The manifold.wasm and manifold.js files do not need to be copied into the project any more, so we can remove
"2. Now build the manifold library and copy the manifold.js and manifold.wasm files into the src/webview/src/wasm/ directory." from the readme. (But no worries, I can do this at a later time.)

Looking great!

Once the new manifold-3d package is released we can clean this up and get it merged.
(Just remove the tar archive file and make sure it is removed from git history I guess.)

[Loosetooth]

Thanks a lot for all the work @tonious !

[tonious]

Now updated against this mega-refactor.

[napter]

@tonious I tried to send you an email but perhaps it went to spam - @Loosetooth and I would be interested in a short zoom/meet just to say hi and introduce ourselves. @elalish same goes for you if you are interested!

[elalish]

Oh yeah, and would be great to finally meet @tonious too. Only trouble is I'm on NZ time. What time zone are the rest of you in?

[napter]

Oh yeah, and would be great to finally meet @tonious too. Only trouble is I'm on NZ time. What time zone are you in @tonious ?

I'm on Eastern (GMT -5) and @Loosetooth is CET/GMT+1? So 3:00 pm my time (15:00), is 9:00 NZ and 9:00 pm (21:00) for Bart. I have the easiest time, being in the middle - what would work for each of you?

[tonious]

Yep it got spamfiled.

I'm in EST as well.

[napter]

Also if either of you want to join, we're discussing 3d coding in our zulip chat (its an open source slack-like chat we host) - here is an invite: https://chat.aptr.io/join/xkoc744gvajxnsuww53fx4va/

[elalish]

Thanks for the invite. I can do 9am on Saturday or Sunday, so Friday or Saturday afternoon/evening for you all. Either of those work?

[Loosetooth]

Thanks for the invite. I can do 9am on Saturday or Sunday, so Friday or Saturday afternoon/evening for you all. Either of those work?

Those times work for me.

[tonious]

Friday is out for me, but Saturday works.

[tonious]

...I can do after 4:30pm on Friday, which should be 10:30am/pm if it doesn't throw anybody off.

[napter]

I can do 4:30 EST Friday / 10:30 Saturday NZ. If that works for everyone can you email me at nathan@cookiecad and I'll share a meeting link?

[napter]

@elalish are you able to join us in four hours?

[elalish]

Sure, did you get my email? I haven't seen an invite yet. elalish@gmail

[napter]

Sure, did you get my email? I haven't seen an invite yet

Just sent!

[elalish]

Now that v3.3 is released, should we merge this? I'd like to try this VSCode extension out more now that we can import libraries.

[Loosetooth]

Hmmm, this requires more work, because there have been quite some extra changes to ManifoldCAD after it became clear that dependencies/packages could be auto-imported/bundled in the browser.

So if we want to support exact ManifoldCAD scripts inside the VS Code extension...
The issues I see:

1. Get types auto-imported in the vscode tab with the script open. Before I used the triple-slash import hack, since we just needed the manifold-3d types. But now we - ideally - would need any other imported package's types too. In ManifoldCAD the monaco-editor-auto-typings package is used. I don't know how/if this can be made to work inside vs code.
2. All ManifoldCAD functions and classes are now imported via manifold-3d/manifoldCAD. We will have to change the extension to somehow automatically provide these types in the .mfc editor inside vs code.

[elalish]

Considering VScode is the monico editor, hopefully the same process can be followed more or less. I'm not sure how VScode exposes its internal editor API to extensions.

[tonious]

Model import might take a little more work. I think manifoldCAD will have to run at the extension level, then hand off the finished model to the webview.

[tonious]

I think this is going to be amazing for larger projects. With dependencies installed via npm, vscode will already be able to find those types.

[Loosetooth]

I think this is going to be amazing for larger projects. With dependencies installed via npm, vscode will already be able to find those types.

Did you try the extension in a "real npm" project @tonious?

The way I saw it was that for a real javascript/typescript project (not ManifoldCAD script) the project should just be bundled and sent to the extension. The extension then executes the bundle, and shows the result in the webview / model-viewer.

This should be quite simple to implement.

Model import might take a little more work. I think manifoldCAD will have to run at the extension level, then hand off the finished model to the webview.

Yes, just read the model files from disk or fetch them, and execute the given "project" bundle. Or keep manifoldCAD in the webview, and somehow send the bundle and files to it.

BUT: what about ManifoldCAD scripts? I'll have to look into this further at some point. Would be sad if we cannot support these directly.

[Loosetooth]

the project should just be bundled and sent to the extension.

Or better: the extension can watch the bundled file on disk and execute it on every change.

[elalish]

Or better: the extension can watch the bundled file on disk and execute it on every change.

Watch out, Manifold may be quick, but a big model can still be lot more compute than a normal JS page. I would leave evaluation to an explicit activation.

[tonious]

Did you try the extension in a "real npm" project @tonious?

I've got a local test project, but I haven't pushed it very far.

The way I saw it was that for a real javascript/typescript project (not ManifoldCAD script) the project should just be bundled and sent to the extension. The extension then executes the bundle, and shows the result in the webview / model-viewer.

Yeah, exactly this. So far, in this PR, the extension hands a single file off to the webview, which then bundles it ignoring other local files. So it might take a little rejiggering.

Also, I had to import types locally, even when vscode should have found them:

import {Manifold, CrossSection, getCircularSegments} from 'manifold-3d/manifoldCAD';
import type {Manifold as ManifoldType, CrossSection as CrossSectionType, Vec3, Vec2} from 'manifold-3d/manifoldCAD.d.ts';

So there's some kind of packaging detail to sort out on the manifold side.

[tonious]

I spent some time this weekend trying to get manifold running at the extension level, to no avail. Plan B is to allow the worker to request files. Which is not a bad idea anyhow; it'd make eventual memfs (or similar) integration easier if done correctly.