Runtime dependency on `@babel/runtime`?

[pmndrs]

Runtime dependency on `@babel/runtime`?

[codynova]

I believe that's in the World class file, it's an older attempt to polyfill. I will take a look soon if Marco doesn't beat me to it.

[marcofugaro]

Yup, @babel/runtime wasn't in dependencies as explained in the documentation.

However we don't need that plugin, since it only ends up saving ~100 bytes.

Should be fixed in #24, @donmccurdy you can check if it works for now using react-spring/cannon-es#fix-babel as a version in the package.json.

[codynova]

Released in [email protected]

[donmccurdy]

I would try to avoid eval and Function(), which will break for users who need a Content Security Policy.

The answer may depend on your build setup. In some cases you can configure it to output both Node.js- and Browser-friendly builds, see How to write a JavaScript package for both Node and the browser. This may be more trouble than you want for a profiling dependency.

You could also try just putting it in an if() block. Say:

let performance;
if (typeof window !== 'undefined') {
  performance = window.performance;
} else {
  performance = require('perf_hooks').performance;
}

As long as the user's build system doesn't bundle a perf_hooks implementation automatically (some bundlers do this for node.js dependencies, I think?), that shouldn't affect build size.

Finally, you could just require node.js users to provide a reference to the object when they enable profiling. Mildly inconvenient, but predictable, and unlikely to affect the majority of users.

[marcofugaro]

Yes I've found a solution in #38.

It avoids us from using any polyfill.

[donmccurdy]

Thanks for the quick fix! :)

I did have one error after that change:

> [email protected] test /Users/donmccurdy/Documents/Projects/three-to-cannon
> node test/index.js

/Users/donmccurdy/Documents/Projects/three-to-cannon/node_modules/cannon-es/dist/cannon-es.cjs.js:13030
  performance = {};
              ^

ReferenceError: performance is not defined
    at Object.<anonymous> (/Users/donmccurdy/Documents/Projects/three-to-cannon/node_modules/cannon-es/dist/cannon-es.cjs.js:13030:15)
    at Module._compile (internal/modules/cjs/loader.js:936:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:947:10)
    at Module.load (internal/modules/cjs/loader.js:790:32)
    at Function.Module._load (internal/modules/cjs/loader.js:703:12)
    at Module.require (internal/modules/cjs/loader.js:830:19)
    at require (internal/modules/cjs/helpers.js:68:18)
    at Object.<anonymous> (/Users/donmccurdy/Documents/Projects/three-to-cannon/dist/three-pathfinding.js:1:7)
    at Module._compile (internal/modules/cjs/loader.js:936:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:947:10)
npm ERR! Test failed.  See above for more details.

This is only in my test suite, which runs in node.js, and was easy enough to patch. If you have users who are actually using cannon-es serverside, they may need the same fix. I don't see the related code in this project, perhaps it's generated in the build step somehow.

// Before importing `cannon-es` in node.js:
const {performance} = require('perf_hooks');
global.performance = performance;

[codynova]

Hey @donmccurdy, I'm finally getting around to this. What do you think is the best solution, considering you need to require('perf_hooks') in Node? I can see this as an option, but not sure if there is a better way:

// Check if global window object exists to understand runtime context (browser vs node)
const hasWindow = new Function('try {return this===window;}catch(e){return false;}')

// performance.now()
const performance = hasWindow() ? (window.performance || {}) : (global.performance || {})

if (!performance.now) {
  let nowOffset = Date.now()
  if (performance.timing && performance.timing.navigationStart) {
    nowOffset = performance.timing.navigationStart
  }
  performance.now = () => Date.now() - nowOffset
}

[codynova]

Finally, you could just require node.js users to provide a reference to the object when they enable profiling. Mildly inconvenient, but predictable, and unlikely to affect the majority of users.

Thank you for the suggestions. I'd honestly rather rely on the existing polyfill than breaking the isomorphic API parity. The conditional require should be fine, I think perf_hooks is pretty tiny.

Fixed in #35 and released in [email protected]. Please let me know if you see any other issues.

[codynova]

This solution (#35) causes an issue with webpack builds #37

I'm not sure of the most elegant isomorphic solution. As mentioned above, we can manually polyfill performance in Node environments, or change the cannon-es API for Node environments to require passing in the perf_hooks module.

@marcofugaro Do you have any ideas for this?

[codynova]

Released fix in [email protected]