Closed kutu closed 11 months ago
jrouwe
commented
11 months ago Sorry, I indeed cannot help you here.
Maybe @isaac-mason has an opinion on swapping default and types (he added TS support)?
The await import("module") and new URL("./",import.meta.url) statements are created by emscripten, so I'm not sure why they would cause a problem.
kutu
commented
11 months ago i was digging this today, and found out that js files you provide are ESM files, means they need to be executed in html with <script type="module"
I'm looking for the old way, which is called CommonJS
Emscripten compiles it as "module" using this args https://github.com/jrouwe/JoltPhysics.js/blob/9c086477c412fe42f7e30c7e938bd9ddd9a0bfce/CMakeLists.txt#L49-L51
If you could provide another bundles along side with "module" one, but without EXPORT_ES6 and with/without MODULARIZE, people usually make filenames like
mylib.es.js
mylib.umd.js
mylib.cjs.js
I tried to build this myself, but I'm on Windows, after successfully executed this command
cmake -B Build/Distribution -DCMAKE_BUILD_TYPE=Distribution
the next one
cmake --build Build/Distribution
failed with error
MSBuild version 17.8.3+195e7f5a3 for .NET Framework
Checking Build System
Generating JoltPhysics.js bindings
Building Custom Rule H:/1/JoltPhysics.js/CMakeLists.txt
Building JoltPhysics.js bindings
Building Custom Rule H:/1/JoltPhysics.js/Build/Distribution/_deps/joltphysics-src/Build/CMakeLists.txt
cl : command line error D8021: invalid numeric argument '/Werror' [H:\1\JoltPhysics.js\Build\Distribution\_deps\joltphysics-build\Jolt.vcxproj]i went to
H:\1\JoltPhysics.js\Build\Distribution\_deps\joltphysics-src\Build\CMakeLists.txt
and commented line 134
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Werror")
and get this error
isaac-mason
commented
11 months ago Looks like the typescript docs do say "types" should come first, I can cut a PR to fix this. It seems that not all bundlers complain about this.
The "types" condition should always come first in "exports".
isaac-mason
commented
11 months ago Re adding a CommonJS build, I'm happy to help with this if there's interest!
I suggested starting with only publishing ESM as it's the modern format, and there is some effort in supporting both.
isaac-mason
commented
11 months ago Also re your immediate issue, what version of webpack are you using? And do you mind sharing your config?
The latest versions of webpack (iirc 4 and 5) should support ESM.
kutu
commented
11 months ago I use latest webpack
This is minimal example to reproduce the problems example.zip
> npm i
> npm run buildafter that you will need to fix types and default order to get the second error
isaac-mason
commented
11 months ago Thanks! I'll take a look soon
isaac-mason
commented
11 months ago I've found that with the following changes, the minimal webpack repro builds successfully:
node from the emcc ENVIRONMENT argument
-s ENVIRONMENT='web,node'-s ENVIRONMENT='web'It appears this isn't an issue with commonjs vs esm, but an issue with webpack not handling the node-specific code that emscripten adds when node is included in the ENVIRONMENT argument.
In another emscripten package recast-navigation-js, I use the argument -s ENVIRONMENT='web' (not specifying node), and the package works fine in node environments.
I'll do some testing later today checking if node is unaffected by this change. If it still works ok, I'll create a PR which should resolve this webpack issue 🙂
isaac-mason
commented
11 months ago With the above change, the wasm-compat (default) flavour works with node, but the wasm flavour breaks, seemingly because node's built-in fetch method is not fully implemented?
node:internal/deps/undici/undici:11372
Error.captureStackTrace(err, this);
^
TypeError: fetch failed
at Object.fetch (node:internal/deps/undici/undici:11372:11) {
cause: Error: not implemented... yet...
at makeNetworkError (node:internal/deps/undici/undici:4801:35)
at schemeFetch (node:internal/deps/undici/undici:9531:18)
at node:internal/deps/undici/undici:9409:26
at mainFetch (node:internal/deps/undici/undici:9428:11)
at fetching (node:internal/deps/undici/undici:9377:7)
at fetch2 (node:internal/deps/undici/undici:9245:20)
at Object.fetch (node:internal/deps/undici/undici:11370:18)
at fetch (node:internal/process/pre_execution:282:25)
at Ia (file:///Users/isaacmason/Development/JoltPhysics.js/dist/jolt-physics.wasm.js:15:133)
at file:///Users/isaacmason/Development/JoltPhysics.js/dist/jolt-physics.wasm.js:43:160
}
Node.js v20.9.0I'll keep investigating!
Reading through some emscripten and webpack issues, having separate node and web builds is a commonly suggested solution 🙁
isaac-mason
commented
11 months ago @kutu I don't have a proper solve yet, but I can share a workaround for using JoltPhysics.js with webpack 5.
These issues appear to be common for emscripten packages that target both web and node environments - I found this workaround from ammo.js github issues discussing similar webpack issues.
Once the fix in #54 is released, this webpack configuration should mitigate the issues webpack has with the node environment-specific code generated by emscripten (see resolve.fallback.module):
const path = require("path");
module.exports = {
mode: "production",
entry: "./main.js",
output: {
filename: "main.js",
path: path.resolve(__dirname, "dist"),
},
resolve: {
fallback: {
module: false,
},
},
};This still outputs one error though:
ERROR in ./node_modules/jolt-physics/dist/jolt-physics.wasm-compat.js 12:178-207
Module not found: Error: Can't resolve './' in '/Users/isaacmason/Downloads/example/node_modules/jolt-physics/dist'
resolve './' in '/Users/isaacmason/Downloads/example/node_modules/jolt-physics/dist'
Parsed request is a directory
using description file: /Users/isaacmason/Downloads/example/node_modules/jolt-physics/package.json (relative path: ./dist)
using description file: /Users/isaacmason/Downloads/example/node_modules/jolt-physics/package.json (relative path: ./dist)
as directory
existing directory /Users/isaacmason/Downloads/example/node_modules/jolt-physics/dist
using description file: /Users/isaacmason/Downloads/example/node_modules/jolt-physics/package.json (relative path: ./dist)
using path: /Users/isaacmason/Downloads/example/node_modules/jolt-physics/dist/index
using description file: /Users/isaacmason/Downloads/example/node_modules/jolt-physics/package.json (relative path: ./dist/index)
no extension
/Users/isaacmason/Downloads/example/node_modules/jolt-physics/dist/index doesn't exist
@ ./main.js 1:0-32 3:19-23This error doesn't prevent the bundle from being generated, and seemingly doesn't break the bundle - jolt-physics can be imported and used fine (I tested with the wasm-compat flavour). To silence this error, you could write a basic plugin, e.g.
const path = require("path");
class FilterErrorsPlugin {
constructor(filter) {
this.filter = new RegExp(filter);
}
apply(compiler) {
compiler.hooks.done.tap("FilterErrorsPlugin", (stats) => {
stats.compilation.errors = stats.compilation.errors.filter((error) => {
return !this.filter.test(error.message);
});
});
}
}
module.exports = {
mode: "production",
entry: "./main.js",
output: {
filename: "main.js",
path: path.resolve(__dirname, "dist"),
},
resolve: {
fallback: {
module: false,
},
},
plugins: [
new FilterErrorsPlugin("Can't resolve '\\./' in '.+/jolt-physics/dist'"),
],
};I recognise these workarounds aren't ideal! As I get time I'll keep investigating what the path forward should be.
Let me know if this helps in the meantime 🙂
kutu
commented
11 months ago yes, it works, thank you for your time.
HunterBarclay
commented
9 months ago @isaac-mason Will there be separate web and node builds made available, or will users just have to use the webpack config fix?
isaac-mason
commented
9 months ago @KyroVibe we don't have a clear path forward for this yet. Doubling the amount of builds we need to support isn't ideal.
For the moment using the above fix is still what I'd recommend. Or if you really need an environment specific build, you can build JoltPhysics.js yourself with different configuration.
HunterBarclay
commented
9 months ago @isaac-mason Yeah, I've just been building it myself so far. Thank you
getting error
fixing it by modifing
package.jsonfromto
defaultis last now (why? dont know)then getting error
I guess webpack doesn't like this parts
and
If I don't
import Joltinto my code, then I will not have auto-completion.I did read that you are not into JS world, so asking other people who can help here. Thanks.