Skip to content

Adds a node-compatible export#12

Open
gkjohnson wants to merge 6 commits into
repalash:masterfrom
gkjohnson:node-version
Open

Adds a node-compatible export#12
gkjohnson wants to merge 6 commits into
repalash:masterfrom
gkjohnson:node-version

Conversation

@gkjohnson

@gkjohnson gkjohnson commented Oct 16, 2024

Copy link
Copy Markdown
Contributor

Fix #9

  • Moves the node variant of the unwrapper to a node.ts file.
  • Adds a separate node entry to the webpack build so two entry points are built (one for browser, one for node)
  • Adds a second import path xatlas-three/node so the node variant can be used easily.
  • Automatically finds the xatlasjs files and initializes the wasm paths so there's no need to call "loadLibrary" explicitly. Unfortunately I don't think this can be done for the browser variant easily due to the state of workers and wasm imports...
  • Add "xatlasjs" dependency since it's required for node support, now.
  • Await the library to be loaded in the base unwrapper

Once node is working correctly there are a few things I'd like to adjust in future PRs if that's okay - let me know how this sounds:

  • Add a "verbose" flag and / or log callback to enable / disable the automatic logging that occurs when loading the xatlasjs files.
  • Add a "dispose" function to dispose of the wasm memory and workers used by the unwrappers.
  • Remove the need to explicitly pass in three.js' BufferAttribute to the constructor.

Comment thread src/node.ts
Comment on lines +11 to +12
constructor(...args:any[]) {
super(...args);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typescript seems to not like this spread syntax for passing through the arguments to the parent constructor. I looked it up but there didn't seem to be a clear solution. Do we have to reproduce all the arguments and types for this constructor?

@gkjohnson

Copy link
Copy Markdown
Contributor Author

I'm also seeing this warning when running the node instance - looks like there's maybe a memory leak in xatlasjs? Or "comlink"?

(node:91465) MaxListenersExceededWarning: Possible EventTarget memory leak detected. 11 message listeners added to [MessagePort [EventTarget]]. Use events.setMaxListeners() to increase limit
    at [kNewListener] (node:internal/event_target:534:17)
    at MessagePort.<anonymous> (node:internal/worker/io:308:12)
    at MessagePort.addEventListener (node:internal/event_target:645:23)
    at /Users/garrett/Desktop/git/xatlas-three/node_modules/comlink/dist/umd/comlink.js:331:16
    at new Promise (<anonymous>)
    at requestResponseMessage (/Users/garrett/Desktop/git/xatlas-three/node_modules/comlink/dist/umd/comlink.js:329:16)
    at Object.apply (/Users/garrett/Desktop/git/xatlas-three/node_modules/comlink/dist/umd/comlink.js:257:24)
    at _onAtlasProgress (/Users/garrett/Desktop/git/xatlas-three/node_modules/xatlasjs/dist/node/xatlas.js:9:63204)
    at wasm://wasm/0008d70a:wasm-function[31]:0xbf8
    at wasm://wasm/0008d70a:wasm-function[133]:0xdfc7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Instructions for running in Node.js?

1 participant