Excalibur icon indicating copy to clipboard operation
Excalibur copied to clipboard

Investigate replacing/changing `instanceof` checks

Open eonarheim opened this issue 2 years ago • 1 comments

Steps to Reproduce

Using instanceof internally in Excalibur has been a source of bugs depending on the bundle/minifier. Vite and Angular appear to need additional configuration to work around their default minification settings so they preserve constructor names.

See

  • https://github.com/excaliburjs/excalibur-tiled/issues/344#issuecomment-1098702780
  • https://github.com/excaliburjs/Excalibur/discussions/2434

Expected Result

Excalibur should be resilient to build tool minification steps. At minimum there should be a warning logged with documentation linked when an instanceof check fails.

Actual Result

Excalibur silently fails and users are mystified to what is wrong.

Environment

  • browsers and versions: N/A
  • operating system: N/A
  • Excalibur versions: v0.27.0

Current Workaround

Update used bundler configuration to preserve names

eonarheim avatar Jul 27 '22 23:07 eonarheim

Perhaps this useful insight: I ran into instanceof not working properly when npm linking @excaliburjs/plugin-tiled to my project. This caused the tilemap to not render anything at all, exactly like #344. The problem ended up being multiple excalibur installs in node_modules, rather than anything with minification. This is a common problem with linking, as you might be linking to a project with its devDependencies installed (as was the case for me with @excaliburjs/plugin-tiled), but it could also happen if there are different versions of excalibur installed too.

Since I was using Vite, the solution was to configure a dedupe, ensuring only 1 instance was bundled in my project.

React does some magic to detect possible multiple installs of itself, perhaps there's something to learn from there to do the same? This is the error it throws, though I'm not sure how it gets to throwing it.

mattjennings avatar Jul 28 '22 23:07 mattjennings

This issue hasn't had any recent activity lately and is being marked as stale automatically.

github-actions[bot] avatar Sep 27 '22 00:09 github-actions[bot]

I think we got to the bottom of this, instanceof is okay as long as you have the right peerDependency setup and vite has dedup enabled

eonarheim avatar Feb 12 '24 06:02 eonarheim